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 632945 - rtph264depay in access-unit=true mode does not aggregate the delta unit flag correctly
rtph264depay in access-unit=true mode does not aggregate the delta unit flag ...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal blocker
: 0.10.26
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-10-22 23:05 UTC by Olivier Crête
Modified: 2010-11-01 15:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
rtph264depay: Set the delta flag only if there is no key unit in the buffer (1.36 KB, patch)
2010-10-22 23:06 UTC, Olivier Crête
none Details | Review
Updated patch (1.36 KB, patch)
2010-10-22 23:10 UTC, Olivier Crête
none Details | Review
really updated patch (1.70 KB, patch)
2010-10-22 23:12 UTC, Olivier Crête
none Details | Review
gdppay output that reproduces the bug. (6.50 KB, application/octet-stream)
2010-10-31 23:39 UTC, Olivier Crête
  Details
alternative patch (3.84 KB, patch)
2010-11-01 14:12 UTC, Wim Taymans
accepted-commit_now Details | Review

Description Olivier Crête 2010-10-22 23:05:58 UTC
In access-unit=true mode, rtph264depay doesn't aggregate the delta unit correctly. Instead of setting it only if none of the parts are delta units, if set it if any of the parts were.. So in partice, I can find cases where it never sets it.
Comment 1 Olivier Crête 2010-10-22 23:06:38 UTC
Created attachment 173045 [details] [review]
rtph264depay: Set the delta flag only if there is no key unit in the buffer
Comment 2 Olivier Crête 2010-10-22 23:10:33 UTC
Created attachment 173046 [details] [review]
Updated patch

Oops, forgot one place.. Maybe it would be easier to replace last_delta with last_keyunit ...
Comment 3 Olivier Crête 2010-10-22 23:12:19 UTC
Created attachment 173047 [details] [review]
really updated patch
Comment 4 Tim-Philipp Müller 2010-10-31 20:38:43 UTC
Patch looks like it makes sense to me. Agree that using last_jeyunit would make the code easier to follow.

Any chance you could you provide a sample input stream (e.g. in GDP format) that triggers this?
Comment 5 Olivier Crête 2010-10-31 23:39:20 UTC
Created attachment 173610 [details]
gdppay output that reproduces the bug.

This pipeline can reproduce it:
videotestsrc num-buffers=2 ! video/x-raw-yuv, width=160, height=120 ! x264enc option-string=slices=4 ! rtph264pay mtu=500 ! rtph264depay access-unit=1 byte-stream=false ! ffdec_h264 ! fakesink dump=1

GdpPay file produced with:
videotestsrc num-buffers=2 ! video/x-raw-yuv, width=160, height=120 ! x264enc option-string=slices=4 ! rtph264pay mtu=500 ! gdppay ! filesink location=h264-sliced.gdp

You can read it with:
filesrc location=h264-sliced.gdp ! gdpdepay ! rtph264depay access-unit=1 byte-stream=false ! ffdec_h264 ! fakesink dump=1

Without my patch, it outputs nothing, with it, it shows a frame or two.
Comment 6 Mark Nauwelaerts 2010-11-01 09:41:08 UTC
[sample stream seems to consist of AU SEQ PIC SEI IDR-I IDR-I IDR-I IDR-I]

So, basically, the problem is due to AU and/or SEI nal in the access-unit, which are pretty "neutral" and currently considered as delta.  It could then alternatively be fixed by also accepting AU and SEI as keyframe in the NAL_TYPE_IS_KEY macro (which also makes sense).
Comment 7 Olivier Crête 2010-11-01 14:09:05 UTC
I think the same thing happens even if just have a AU split into slices, and it only recognizes the first slice as a keyframe ? Or maybe I read the code wrong.
Comment 8 Wim Taymans 2010-11-01 14:12:05 UTC
Created attachment 173635 [details] [review]
alternative patch

Renames the variable to make things clearer.
Comment 9 Tim-Philipp Müller 2010-11-01 14:55:05 UTC
Comment on attachment 173635 [details] [review]
alternative patch

Thanks!
Comment 10 Wim Taymans 2010-11-01 15:00:50 UTC
commit 706731b331c2aa77b6e3d58d7eb72ebdcdb4ee1b
Author: Wim Taymans <wim.taymans@collabora.co.uk>
Date:   Mon Nov 1 14:56:28 2010 +0100

    rtph264depay: only set delta unit on all-non-key units
    
    Only set the delta flag when all of the units in the packet are delta units.
    Based on patch from Olivier Crête <olivier.crete@collabora.co.uk>
    
    Fixes #632945