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 723284 - decoder: h264: add support for NALU "alignment" optimization
decoder: h264: add support for NALU "alignment" optimization
Status: RESOLVED FIXED
Product: gstreamer-vaapi
Classification: Other
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: gstreamer-vaapi maintainer(s)
gstreamer-vaapi maintainer(s)
Depends on:
Blocks: 720305
 
 
Reported: 2014-01-30 13:43 UTC by Gwenole Beauchesne
Modified: 2014-06-19 13:35 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
decoder: h264: add support for NALU "alignment" optimization (6.01 KB, patch)
2014-02-06 06:37 UTC, sreerenj
none Details | Review

Description Gwenole Beauchesne 2014-01-30 13:43:59 UTC
We can avoid scanning for start codes again if the bitstream is fed in NALU chunks. Currently, we always scan for start codes, and keep track of remaining bits in a GstAdapter, even if, in practice, we are likely receiving one GstBuffer per NAL unit. i.e. h264parse with "nal" alignment.

New APIs:
* gst_vaapi_decoder_h264_set_alignment():
  where alignment = { au(default), nal }
Comment 1 sreerenj 2014-02-05 09:08:06 UTC
What is the real intention behind the new API? I guess you doesn't want to add caps paring inside gst_vaapi_decoder_h264_new() and a clear separation of libgstvaapi from gstreamer dependency in future. isn't it? 

IF so, we need to parse the codecspecific attributes like 
"alignment" with in gst/vaapi/gstvaapidecode.c

Or do you have something else in your mind?
Comment 2 Gwenole Beauchesne 2014-02-05 09:37:24 UTC
An access unit could include several NAL units. This means multiple calls to scan_for_start_code() again in libgstvaapi. If we give a guarantee to the libgstvaapi decoder that every GstBuffer exactly contains a single NAL unit, then we don't need to scan for the next start code prefix again.

Benefits are twofold: (i) you read less bytes in "nal" alignment since h264parse could possibly have prepared that for you already, and (ii) you can reduce latency to decode the current frame, i.e. you no longer need to wait for the next frame start code prefix to trigger a decode of the current frame. I don't believe much in (ii) since that latency is just moved up to h264parse instead. :)

This optimization is only valid for unpacketized streams, e.g. TS.

On the implementation side, yes, the vaapidecode plug-in element needs to parse the GstCaps. Though, I am not decided yet whether we would split the vaapidecode element into multiple instances per-codec, to align with the vaapiencode_* split, or if the support code would simply be put to a separate file but the unique vaapidecode plug-in element kept. Opinions on that?
Comment 3 sreerenj 2014-02-05 10:06:20 UTC
(In reply to comment #2)
> An access unit could include several NAL units. This means multiple calls to
> scan_for_start_code() again in libgstvaapi. If we give a guarantee to the
> libgstvaapi decoder that every GstBuffer exactly contains a single NAL unit,
> then we don't need to scan for the next start code prefix again.

Yes that I understood. In nal aligned byte-stream ,I think what ever bytes available in the adapter for each invocation of parse() routine is equal to the size of last buffer received which has exact one nal unit. Which means we can just assume "buf_size = gst_adapter_available() with in gst_vaapi_decoder_h264_parse()

> 
> On the implementation side, yes, the vaapidecode plug-in element needs to parse
> the GstCaps. Though, I am not decided yet whether we would split the
> vaapidecode element into multiple instances per-codec, to align with the
> vaapiencode_* split, or if the support code would simply be put to a separate
> file but the unique vaapidecode plug-in element kept. Opinions on that?

My preference was a unique vaapidecode plugin but then we should align the encoder also in same way. Personally I didn't like to have a vaapidecode + vaapiencode_*  combination of plugins. What is the difficulty to keep a common vaapiencode?? 
If there are significant reasons to keep vaapiencode as vaapiencode_* then, we can change the vaapidecode also to vaapidecode_* derivatives. 

Another point is that the vaapidecode_* and vaapiencode_* might give more clarity to the customers of gstreamer-vaapi (since it is providing clear indication of the codec in use)


For now I can provide the patch which will do two things in gstvaapidecode.c

--- parse alignment fields from the caps for h264 streams
--- use gst_vaapi_decoder_h264_set_alignment() api after creating the GstVaapiDecoder.
Comment 4 sreerenj 2014-02-06 06:37:05 UTC
Created attachment 268255 [details] [review]
decoder: h264: add support for NALU "alignment" optimization
Comment 5 Gwenole Beauchesne 2014-06-19 13:35:20 UTC
commit cb9f98f0d5501f7f700e33bbc969d0ce420c2601
Author: Sreerenj Balachandran <sreerenj.balachandran@intel.com>
Date:   Thu Feb 6 08:30:10 2014 +0200

    decoder: h264: add support for NALU "alignment" optimization.
    
    We can avoid scanning for start codes again if the bitstream is fed
    in NALU chunks. Currently, we always scan for start codes, and keep
    track of remaining bits in a GstAdapter, even if, in practice, we
    are likely receiving one GstBuffer per NAL unit. i.e. h264parse with
    "nal" alignment.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=723284
    
    [use gst_adapter_available_fast() to determine the top buffer size]
    Signed-off-by: Gwenole Beauchesne <gwenole.beauchesne@intel.com>