[Spice-devel] [PATCH spice-gtk 1/2] channel-display: keep one struct array for video-codecs data
Christophe Fergeau
cfergeau at redhat.com
Tue Jul 11 11:59:37 UTC 2017
On Mon, Jul 10, 2017 at 05:52:06PM +0200, Victor Toso wrote:
> From: Victor Toso <me at victortoso.com>
>
> On channel-display.c, for each SpiceVideoCodecType we need:
> - Its enum type;
> - The associated capability;
> - A name;
>
> On channel-display-gst.c, for each SpiceVideoCodecType we need:
> - Associated decoding elements for the pipeline;
> - Associated GstCaps, also for the video pipeline;
>
> Follow up patch will need the associated name for given
> SpiceVideoCodecType.
>
> Moving gst_opts[] array to channel-display-priv.h to be reused. This
> should also make slightly simpler when supporting a new video codec in
> the future.
>
> Signed-off-by: Victor Toso <victortoso at redhat.com>
> ---
> src/channel-display-gst.c | 36 ------------------------------------
> src/channel-display-priv.h | 43 +++++++++++++++++++++++++++++++++++++++++++
> src/channel-display.c | 18 ++++--------------
> 3 files changed, 47 insertions(+), 50 deletions(-)
>
> diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> index 8669562..acea4a2 100644
> --- a/src/channel-display-gst.c
> +++ b/src/channel-display-gst.c
> @@ -50,42 +50,6 @@ typedef struct SpiceGstDecoder {
> guint timer_id;
> } SpiceGstDecoder;
>
> -/* FIXME: With gstreamer version 1.9.0 and higher, we are using playbin to
> - * create the pipeline for us and for that reason we don't need to keep track of
> - * decoder's name anymore. */
> -static struct {
> - const gchar *dec_name;
> - const gchar *dec_caps;
> -} gst_opts[] = {
> - /* SpiceVideoCodecType starts at index 1 */
> - { NULL, NULL },
> -
> - /* SPICE_VIDEO_CODEC_TYPE_MJPEG */
> - { "jpegdec", "image/jpeg" },
> -
> - /* SPICE_VIDEO_CODEC_TYPE_VP8
> - *
> - * typefind is unable to identify VP8 streams by design.
> - * See: https://bugzilla.gnome.org/show_bug.cgi?id=756457
> - */
> - { "vp8dec", "video/x-vp8" },
> -
> - /* SPICE_VIDEO_CODEC_TYPE_H264
> - * When setting video/x-h264, h264parse will complain if we don't have the
> - * stream-format or codec_data information. As stream-format is byte-stream
> - * (hardcoded in spice-server), let's add it here to avoid the warning.
> - */
> - { "h264parse ! avdec_h264", "video/x-h264,stream-format=byte-stream" },
> -
> - /* SPICE_VIDEO_CODEC_TYPE_VP9 */
> - { "vp9dec", "video/x-vp9" },
> -};
> -
> -G_STATIC_ASSERT(G_N_ELEMENTS(gst_opts) <= SPICE_VIDEO_CODEC_TYPE_ENUM_END);
> -
> -#define VALID_VIDEO_CODEC_TYPE(codec) \
> - (codec > 0 && codec < G_N_ELEMENTS(gst_opts))
> -
> /* GstPlayFlags enum is in plugin's header which should not be exported.
> * https://bugzilla.gnome.org/show_bug.cgi?id=784279
> */
> diff --git a/src/channel-display-priv.h b/src/channel-display-priv.h
> index 3c9d119..00f71f8 100644
> --- a/src/channel-display-priv.h
> +++ b/src/channel-display-priv.h
> @@ -150,6 +150,49 @@ struct display_stream {
> uint32_t report_drops_seq_len;
> };
>
> +static const struct {
> + int cap;
> + const gchar name[8];
> +
> + /* FIXME: With gstreamer version 1.9.0 and higher, we are using playbin to
> + * create the pipeline for us and for that reason we don't need to keep track of
> + * decoder's name anymore. */
> + const gchar *dec_name;
> + const gchar *dec_caps;
> +} gst_opts[] = {
> + /* SpiceVideoCodecType starts at index 1 */
> + { 0 },
> +
> + /* SPICE_VIDEO_CODEC_TYPE_MJPEG */
> + { SPICE_DISPLAY_CAP_CODEC_MJPEG, "mjpeg",
> + "jpegdec", "image/jpeg" },
> +
> + /* SPICE_VIDEO_CODEC_TYPE_VP8
> + *
> + * typefind is unable to identify VP8 streams by design.
> + * See: https://bugzilla.gnome.org/show_bug.cgi?id=756457
> + */
> + { SPICE_DISPLAY_CAP_CODEC_VP8, "vp8",
> + "vp8dec", "video/x-vp8" },
> +
> + /* SPICE_VIDEO_CODEC_TYPE_H264
> + * When setting video/x-h264, h264parse will complain if we don't have the
> + * stream-format or codec_data information. As stream-format is byte-stream
> + * (hardcoded in spice-server), let's add it here to avoid the warning.
> + */
> + { SPICE_DISPLAY_CAP_CODEC_H264, "h264",
> + "h264parse ! avdec_h264", "video/x-h264,stream-format=byte-stream" },
> +
> + /* SPICE_VIDEO_CODEC_TYPE_VP9 */
> + { SPICE_DISPLAY_CAP_CODEC_VP9, "vp9",
> + "vp9dec", "video/x-vp9" },
> +};
> +
> +G_STATIC_ASSERT(G_N_ELEMENTS(gst_opts) <= SPICE_VIDEO_CODEC_TYPE_ENUM_END);
> +
> +#define VALID_VIDEO_CODEC_TYPE(codec) \
> + (codec > 0 && codec < G_N_ELEMENTS(gst_opts))
> +
VALID_VIDEO_CODEC_TYPE could stay in the .c file I believe.
> guint32 stream_get_time(display_stream *st);
> void stream_dropped_frame_on_playback(display_stream *st);
> void stream_display_frame(display_stream *st, SpiceFrame *frame, uint32_t width, uint32_t height, uint8_t* data);
> diff --git a/src/channel-display.c b/src/channel-display.c
> index 06c503c..e51064a 100644
> --- a/src/channel-display.c
> +++ b/src/channel-display.c
> @@ -764,16 +764,6 @@ static HDC create_compatible_dc(void)
> static void spice_display_channel_reset_capabilities(SpiceChannel *channel)
> {
> guint i;
> - static const struct {
> - SpiceVideoCodecType type;
> - int cap;
> - const gchar name[8];
> - } gst_codecs[] = {
> - {SPICE_VIDEO_CODEC_TYPE_MJPEG, SPICE_DISPLAY_CAP_CODEC_MJPEG, "mjpeg"},
> - {SPICE_VIDEO_CODEC_TYPE_VP8, SPICE_DISPLAY_CAP_CODEC_VP8, "vp8"},
> - {SPICE_VIDEO_CODEC_TYPE_H264, SPICE_DISPLAY_CAP_CODEC_H264, "h264"},
> - {SPICE_VIDEO_CODEC_TYPE_VP9, SPICE_DISPLAY_CAP_CODEC_VP9, "vp9"},
> - };
>
> spice_channel_set_capability(SPICE_CHANNEL(channel), SPICE_DISPLAY_CAP_SIZED_STREAM);
> spice_channel_set_capability(SPICE_CHANNEL(channel), SPICE_DISPLAY_CAP_MONITORS_CONFIG);
> @@ -792,11 +782,11 @@ static void spice_display_channel_reset_capabilities(SpiceChannel *channel)
> #ifdef HAVE_BUILTIN_MJPEG
> spice_channel_set_capability(SPICE_CHANNEL(channel), SPICE_DISPLAY_CAP_CODEC_MJPEG);
> #endif
> - for (i = 0; i < G_N_ELEMENTS(gst_codecs); i++) {
> - if (gstvideo_has_codec(gst_codecs[i].type)) {
> - spice_channel_set_capability(SPICE_CHANNEL(channel), gst_codecs[i].cap);
> + for (i = 1; i < G_N_ELEMENTS(gst_opts); i++) {
> + if (gstvideo_has_codec(i)) {
The explicit "type" field made things feel safer, but ok.
Acked-by: Christophe Fergeau <cfergeau at redhat.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170711/6e75a92a/attachment.sig>
More information about the Spice-devel
mailing list