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 722288 - avviddec: mark key frames as sync points
avviddec: mark key frames as sync points
Status: RESOLVED NOTABUG
Product: GStreamer
Classification: Platform
Component: gst-libav
git master
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-01-15 19:38 UTC by Aleix Conchillo Flaqué
Modified: 2014-02-04 11:42 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
mark key frames as sync points (1.30 KB, patch)
2014-01-15 19:40 UTC, Aleix Conchillo Flaqué
rejected Details | Review

Description Aleix Conchillo Flaqué 2014-01-15 19:38:08 UTC
Decoded video frames are not marked as sync points. This could be useful to mark buffers as corrupted, in the base videodecoder, if no sync point has been received before.
Comment 1 Aleix Conchillo Flaqué 2014-01-15 19:40:34 UTC
Created attachment 266384 [details] [review]
mark key frames as sync points

A proposal to mark frames as sync point if they are a key frame.
Comment 2 Sebastian Dröge (slomo) 2014-01-15 19:48:18 UTC
How is that useful currently? The flag will be set but just be ignored by the base class at that point IIRC?
Comment 3 Aleix Conchillo Flaqué 2014-01-15 19:53:56 UTC
(In reply to comment #2)
> How is that useful currently? The flag will be set but just be ignored by the
> base class at that point IIRC?

Just filed bug 722290.
Comment 4 Olivier Crête 2014-01-15 20:05:36 UTC
Isn't the flag already set on the GstVideoCodecFrame based on the flags on the incoming GstBuffer ?
Comment 5 Sebastian Dröge (slomo) 2014-01-15 20:06:28 UTC
Yes, unless something else is wrong or it wasn't properly set before.
Comment 6 Aleix Conchillo Flaqué 2014-01-15 20:14:49 UTC
(In reply to comment #4)
> Isn't the flag already set on the GstVideoCodecFrame based on the flags on the
> incoming GstBuffer ?

If I remove this patch and keep the one in bug 722290, all buffers as marked as corrupted (as no sync point has been received).

I'll double check where this sync point flag is dropped.
Comment 7 Aleix Conchillo Flaqué 2014-01-15 20:48:37 UTC
I just filed a bug to libav to see if it is possible to add information to know if frames can be decoded even the reference key frame is not present.

https://bugzilla.libav.org/show_bug.cgi?id=628
Comment 8 Sebastian Dröge (slomo) 2014-01-16 12:57:16 UTC
Comment on attachment 266384 [details] [review]
mark key frames as sync points

As discussed this seems rather like a workaround for a problem elsewhere. The flag should already have been set on the input side of the encoder. Need to check why it gets lost
Comment 9 Aleix Conchillo Flaqué 2014-01-17 23:38:36 UTC
(In reply to comment #8)
> (From update of attachment 266384 [details] [review])
> As discussed this seems rather like a workaround for a problem elsewhere. The
> flag should already have been set on the input side of the encoder. Need to
> check why it gets lost

This seems to work fine.

However, when using the intra-refresh option in x264, there are no key frames and all encoded frames are considered delta units, so sync point is not set on any frame.

libav considers the frame with the first intra block column to be a key frame (as set in AVFrame->key_frame. AVFrame->picture_type still says it's a p-frame).

So, I guess we could mark this as INVALID?
Comment 10 Sebastian Dröge (slomo) 2014-01-18 09:22:49 UTC
Ah, and h264parse sets the DELTA flag on all frames for intra-refresh streams? It should probably do the same as libav here, even if it is arguably not strictly correct.
Comment 11 Aleix Conchillo Flaqué 2014-01-20 20:41:13 UTC
(In reply to comment #10)
> Ah, and h264parse sets the DELTA flag on all frames for intra-refresh streams?

Yes, it seems that way.

I haven't tried h264parse, but rtph264depay seems to have the same "problem".

> It should probably do the same as libav here, even if it is arguably not
> strictly correct.

I'll see if I find some time to look at this.
Comment 12 Sebastian Dröge (slomo) 2014-02-04 11:42:12 UTC
Let's close this then. Can you file a new bug on h264parse/rtph264depay and describe the situation there and what should be done?