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 580271 - videorate: fails to clear discont flag on duplicated buffers
videorate: fails to clear discont flag on duplicated buffers
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal blocker
: 0.10.23
Assigned To: Wim Taymans
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2009-04-26 10:05 UTC by Arek Korbik
Modified: 2009-04-28 14:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
handle DISCONT correctly (1.62 KB, patch)
2009-04-27 14:57 UTC, Wim Taymans
committed Details | Review

Description Arek Korbik 2009-04-26 10:05:21 UTC
Running the pipeline with HEAD (the same with ubuntu's gst 0.10.22-1, -base 0.10.22-5):

  gst-launch-0.10 videotestsrc pattern=12 num-buffers=4 ! video/x-raw-yuv,framerate=1/1 ! videorate ! video/x-raw-yuv,framerate=30/1 ! theoraenc ! queue ! oggmux ! filesink location=/tmp/chopped.ogg

produces a file that has the following structure (note the non-consecutive page numbers between 1 and 29):

|1809875873| page 0, GP            0 [      0], Flags: B (1 (42))
|1809875873|   GP 0|0,  packetno 0: 42 bytes
|1809875873| page 1, granulepos 0|0, Flags:  (2 (63))
|1809875873|   GP -1|63,  packetno 1: 50 bytes
|1809875873|   GP 0|0,  packetno 2: 2613 bytes
|1809875873| page 3, granulepos 1|0, Flags:  (1 (8))
|1809875873|   GP 1|0,  packetno 4: 8 bytes
|1809875873| page 5, granulepos 1|1, Flags:  (1 (9))
|1809875873|   GP 1|1,  packetno 6: 9 bytes
|1809875873| page 7, granulepos 1|2, Flags:  (1 (9))
|1809875873|   GP 1|2,  packetno 8: 9 bytes
|1809875873| page 9, granulepos 1|3, Flags:  (1 (9))
|1809875873|   GP 1|3,  packetno 10: 9 bytes
|1809875873| page 11, granulepos 1|4, Flags:  (1 (9))
|1809875873|   GP 1|4,  packetno 12: 9 bytes
|1809875873| page 13, granulepos 1|5, Flags:  (1 (9))
|1809875873|   GP 1|5,  packetno 14: 9 bytes
|1809875873| page 15, granulepos 1|6, Flags:  (1 (9))
|1809875873|   GP 1|6,  packetno 16: 9 bytes
|1809875873| page 17, granulepos 1|7, Flags:  (1 (9))
|1809875873|   GP 1|7,  packetno 18: 9 bytes
|1809875873| page 19, granulepos 1|8, Flags:  (1 (9))
|1809875873|   GP 1|8,  packetno 20: 9 bytes
|1809875873| page 21, granulepos 1|9, Flags:  (1 (9))
|1809875873|   GP 1|9,  packetno 22: 9 bytes
|1809875873| page 23, granulepos 1|10, Flags:  (1 (9))
|1809875873|   GP 1|10,  packetno 24: 9 bytes
|1809875873| page 25, granulepos 1|11, Flags:  (1 (9))
|1809875873|   GP 1|11,  packetno 26: 9 bytes
|1809875873| page 27, granulepos 1|12, Flags:  (1 (9))
|1809875873|   GP 1|12,  packetno 28: 9 bytes
|1809875873| page 29, granulepos 1|13, Flags:  (1 (9))
|1809875873|   GP 1|13,  packetno 30: 9 bytes
|1809875873| page 30, granulepos 1|14, Flags:  (1 (9))
|1809875873|   GP 1|14,  packetno 31: 9 bytes
|1809875873| page 31, granulepos 1|15, Flags:  (1 (54))
|1809875873|   GP 1|15,  packetno 32: 54 bytes
...

(after that the page numbers are consecutive).

The same pipeline produces correct/continuous ogg with (ubuntu's) gstreamer 0.10.21-4 and -base 0.10.21-3.
Comment 1 Wim Taymans 2009-04-27 14:33:17 UTC
videorate duplicates the first frame with the DISCONT flag multiple times, oggmux encodes the discont by inserting a discont in the packet numbers.
Comment 2 Wim Taymans 2009-04-27 14:57:30 UTC
Created attachment 133422 [details] [review]
handle DISCONT correctly

Set the DISCONT flag correctly on outgoing buffer. We produce discont as the first buffer and after a flushing seek.
Comment 3 Wim Taymans 2009-04-27 15:39:17 UTC
I can't see how this is a regression at all. No commits touched those parts in any meaningful way since the last release. Making non-critical until we can figure out what broke it.
Comment 4 Zaheer Abbas Merali 2009-04-27 15:46:58 UTC
it's obviously a regression since 0.10.21....
Comment 5 Wim Taymans 2009-04-27 16:18:48 UTC
any idea what change did it then? I can't find anything..
Comment 6 Zaheer Abbas Merali 2009-04-27 17:39:32 UTC
f93fcafe6489f70a8ae5d3f379ed8c2b8ed12018 is first bad commit
commit f93fcafe6489f70a8ae5d3f379ed8c2b8ed12018
Author: Wim Taymans <wim.taymans@gmail.com>
Date:   Tue Nov 25 15:14:30 2008 +0000

    ext/theora/: Parse segment events.
    
    Original commit message from CVS:
    * ext/theora/gsttheoraenc.h:
    * ext/theora/theoraenc.c: (gst_theora_enc_init),
    (theora_buffer_from_packet), (theora_push_packet),
    (theora_enc_sink_event), (theora_enc_is_discontinuous),
    (theora_enc_chain):
    Parse segment events.
    Pass incomming buffer timestamps to outgoing buffers.
    Use the running_time to construct the granulepos.
    Fixes #562163.

is the cause, I did a git bisect. Marking as blocker as the commit that broke it is found
Comment 7 Jan Schmidt 2009-04-27 18:46:59 UTC
I guess that's the corresponding commit to the one that broke vorbisenc
Comment 8 Wim Taymans 2009-04-28 08:52:59 UTC
Can you verify that the videorate patch also fixes it, because that theoraenc patch should be the right one to do.
Comment 9 Wim Taymans 2009-04-28 08:54:16 UTC
the vorbienc commit is a different thing..
Comment 10 Wim Taymans 2009-04-28 09:02:08 UTC
The thing that broke theoraenc in that commit is because it copied the DISCONT flag from the input to the output where previously it would only set a discont if the timestamps differed too much. This cause oggmux to increment the page number to encode the discont.

The videorate patch still seems the best fix.
Comment 11 Zaheer Abbas Merali 2009-04-28 09:10:38 UTC
yah videorate patch also fixes it.
Comment 12 Jan Schmidt 2009-04-28 09:53:52 UTC
Thanks for confirming.
Comment 13 Wim Taymans 2009-04-28 14:49:10 UTC
commit 915b3d139dee6d5ffcc36a635557ca24269b2eb8
Author: Wim Taymans <wim.taymans@collabora.co.uk>
Date:   Tue Apr 28 11:32:49 2009 +0200

    videorate: clear discont on duplicated buffers
    
    When videorate duplicates a buffer with a DISCONT flag, it copies the discont on
    the first pushed buffer but fails to clear it for subsequent buffers. This
    causes theoraenc!oggmux and possibly other elements to consider this a discont
    stream.
    
    Fix videorate to produce discont as the first buffer and after a flushing seek.
    
    Fixes #580271.