After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 779309 - h264parse: Include `interlace-mode` in caps
h264parse: Include `interlace-mode` in caps
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
unspecified
Other All
: Normal enhancement
: 1.11.90
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-02-27 13:38 UTC by Thibault Saunier
Modified: 2017-02-27 19:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
h264parse: Include `interlace-mode` in caps (1.52 KB, patch)
2017-02-27 13:38 UTC, Thibault Saunier
none Details | Review
h264parse: Include `interlace-mode` in caps (1.59 KB, patch)
2017-02-27 13:55 UTC, Thibault Saunier
committed Details | Review

Description Thibault Saunier 2017-02-27 13:38:15 UTC
See commit message
Comment 1 Thibault Saunier 2017-02-27 13:38:20 UTC
Created attachment 346815 [details] [review]
h264parse: Include `interlace-mode` in caps

Those are the rules:

In the SPS:
  * if frame_mbs_only_flag=1 => all frame progressive
  * if frame_mbs_only_flag=0 => field_pic_flag defines if each frame is
    progressive or interlaced, thus the mode is 'mixed' in GStreamer
    terms.
Comment 2 Sebastian Dröge (slomo) 2017-02-27 13:48:18 UTC
Review of attachment 346815 [details] [review]:

::: gst/videoparsers/gsth264parse.c
@@ +1872,3 @@
+
+      gst_caps_set_simple (caps, "interlace-mode", G_TYPE_STRING,
+          gst_video_interlace_mode_to_string (imode), NULL);

Actually no. You should prefer what upstream has given in the caps, if anything, and only otherwise set your own value
Comment 3 Thibault Saunier 2017-02-27 13:49:19 UTC
(In reply to Sebastian Dröge (slomo) from comment #2)
> Review of attachment 346815 [details] [review] [review]:
> 
> ::: gst/videoparsers/gsth264parse.c
> @@ +1872,3 @@
> +
> +      gst_caps_set_simple (caps, "interlace-mode", G_TYPE_STRING,
> +          gst_video_interlace_mode_to_string (imode), NULL);
> 
> Actually no. You should prefer what upstream has given in the caps, if
> anything, and only otherwise set your own value

Indeed, missed that, on it.
Comment 4 Thibault Saunier 2017-02-27 13:55:19 UTC
Created attachment 346818 [details] [review]
h264parse: Include `interlace-mode` in caps

Those are the rules:

In the SPS:
  * if frame_mbs_only_flag=1 => all frame progressive
  * if frame_mbs_only_flag=0 => field_pic_flag defines if each frame is
    progressive or interlaced, thus the mode is 'mixed' in GStreamer
    terms.
Comment 5 Thibault Saunier 2017-02-27 19:34:04 UTC
commit b75a61342f4ea039d922a966f36b02cd9d9c3ad8
Author: Thibault Saunier <thibault.saunier@osg.samsung.com>
Date:   Mon Feb 27 10:32:45 2017 -0300

    h264parse: Include `interlace-mode` in caps
    
    Those are the rules:
    
    In the SPS:
      * if frame_mbs_only_flag=1 => all frame progressive
      * if frame_mbs_only_flag=0 => field_pic_flag defines if each frame is
        progressive or interlaced, thus the mode is 'mixed' in GStreamer
        terms.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=779309