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 692388 - [API] codecparsers: vc1: parse slice headers
[API] codecparsers: vc1: parse slice headers
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.1.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-01-23 13:58 UTC by Gwenole Beauchesne
Modified: 2013-01-24 23:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
codecparsers: vc1: add API to parse slice headers (3.93 KB, patch)
2013-01-23 13:58 UTC, Gwenole Beauchesne
accepted-commit_now Details | Review
updated patch with "Since:" tags (3.99 KB, patch)
2013-01-23 15:30 UTC, Gwenole Beauchesne
committed Details | Review

Description Gwenole Beauchesne 2013-01-23 13:58:23 UTC
Created attachment 234201 [details] [review]
codecparsers: vc1: add API to parse slice headers

Hi, this patch adds a utility function to parse slice headers in VC-1 bitstreams. I just need to know the SLICE_ADDR syntax element, which represents the slice vertical position.

If PIC_HEADER_FLAG is set to one, then a picture layer is parsed but not recorded (through a user-provided struct for example). Why? Well, this is because the picture header in slice header shall be the same as what was previously parsed for the parent frame BDU.
Comment 1 Gwenole Beauchesne 2013-01-23 13:59:15 UTC
Forgot to mention, this fixes SA00049.vc1 conformance test in gstreamer-vaapi, among others probably.
Comment 2 Tim-Philipp Müller 2013-01-23 14:38:03 UTC
Comment on attachment 234201 [details] [review]
codecparsers: vc1: add API to parse slice headers

Looks fine. Perhaps mark the sequence header structure argumen as const  * if it's not going to be touched/changed. And please add a 'Since: 1.2' or 'Since: 1.0.6' at the end of the gtk-doc blob.
Comment 3 Gwenole Beauchesne 2013-01-23 15:30:48 UTC
Created attachment 234216 [details] [review]
updated patch with "Since:" tags
Comment 4 Gwenole Beauchesne 2013-01-23 15:31:11 UTC
(In reply to comment #2)
> (From update of attachment 234201 [details] [review])
> Looks fine. Perhaps mark the sequence header structure argumen as const  * if
> it's not going to be touched/changed. And please add a 'Since: 1.2' or 'Since:
> 1.0.6' at the end of the gtk-doc blob.

Unfortunately, I can't make seqhdr const * because parse_frame_header_advanced() may alter it through calculate_mb_size().

If we want seqhdr to be const * for all VC-1 API, then I think we probably should write some gst_vc1_finalise_sequence_header() similar in sense to gst_mpeg_video_finalise_sequence_header() where we compute the required fields that may be changed subsequently after a sequence header, e.g. frame header. WDYT?

Meanwhile, could I simply push that patch with the "Since:" addition?
Comment 5 Tim-Philipp Müller 2013-01-23 15:42:05 UTC
Ah, I see, nevermind then. It was only for the case that it wouldn't be changed. I think it's fine as it is in this case then.
Comment 6 Tim-Philipp Müller 2013-01-24 22:56:34 UTC
commit 07a51b16eb92e998a831e65a4a54466da29ea469
Author: Gwenole Beauchesne <gwenole.beauchesne@intel.com>
Date:   Wed Jan 23 14:47:58 2013 +0100

    codecparsers: vc1: add API to parse slice headers.
    
    Add gst_vc1_parse_slice_header() function to parse slice headers as
    described in 7.1.2. Slice layers are optional and allowed in advanced
    profile mode only. Picture header, if available (PIC_HEADER_FLAG),
    is parsed but not recorded because it shall be the same as that was
    previously parsed with gst_vc1_parse_frame_header().
    
    This fixes SA00049.vc1 conformance test.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=692388
    
    Signed-off-by: Gwenole Beauchesne <gwenole.beauchesne@intel.com>


(Note that in case you do want this in 1.0.x as well, it would be fine to cherry-pick, since it's fairly trivial API and in -bad, as long as the Since gets fixed up).