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 632549 - [mpeg2dec] answers to position queries are wrong for DVDs
[mpeg2dec] answers to position queries are wrong for DVDs
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-ugly
git master
Other Linux
: Normal normal
: 0.10.17
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-10-19 12:53 UTC by Guillaume Emont (guijemont)
Modified: 2010-10-25 12:57 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
shows the current position every second, according to the pipeline and the decoder (1.50 KB, text/x-python)
2010-10-19 12:53 UTC, Guillaume Emont (guijemont)
  Details
[mpeg2dec] fix position query by trusting upstream (1.32 KB, patch)
2010-10-19 12:55 UTC, Guillaume Emont (guijemont)
committed Details | Review
mpeg2dec: convert the position to stream time before answering to a position query (884 bytes, patch)
2010-10-20 18:45 UTC, Guillaume Emont (guijemont)
committed Details | Review

Description Guillaume Emont (guijemont) 2010-10-19 12:53:48 UTC
Created attachment 172710 [details]
shows the current position every second, according to the pipeline and the decoder

The position returned by mpeg2dec are wrong for DVDs. This can be put in evidence with the attached example to run with a dvd device or iso file as argument.

The example displays every second the position, as returned by the decoder and by the whole pipeline (I guess in that case the sink is answering the query).

When starting playback from any chapter (e.g. through the DVD menus), the position at the start of chapter is at around 10s according to the decoder.

I have a patch that works around this by forwarding the query upstream.
Comment 1 Guillaume Emont (guijemont) 2010-10-19 12:55:40 UTC
Created attachment 172712 [details] [review]
[mpeg2dec] fix position query by trusting upstream

Now ask upstream for position queries, fall back to the old code if upstream cannot answer the query.
Comment 2 Sebastian Dröge (slomo) 2010-10-19 14:32:04 UTC
commit 0fa75d404b3bc1f94e654c3d843637f204fa42f2
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Tue Oct 19 16:30:51 2010 +0200

    mpeg2dec: Use gst_pad_peer_query() instead of getting the peer pad manually

commit 7fb7129a832b4f69800eb4b49ed2a99904ed2e7c
Author: Guillaume Emont <gemont@igalia.com>
Date:   Wed Oct 13 21:38:06 2010 +0200

    mpeg2dec: fix position query by trusting upstream
    
    Position queries are badly handled for DVDs (probably due to the division i
    chapters): the time returned was the time since the start of chapter.
    
    Now ask upstream for position queries, fall back to the old code if upstrea
    cannot answer the query.
Comment 3 Jan Schmidt 2010-10-19 15:10:25 UTC
This is because the resinDVD version of the MPEG PS demuxer outputs all segments with timestamps starting from 10secs. The MPEG decoder is answering the position incorrectly - reporting the current timestamp rather than converting it to the current segment.
Comment 4 Guillaume Emont (guijemont) 2010-10-20 18:45:48 UTC
Created attachment 172867 [details] [review]
mpeg2dec: convert the position to stream time before answering to a position query
Comment 5 Guillaume Emont (guijemont) 2010-10-20 18:47:51 UTC
So I guess this new patch would make more sense? And maybe we could/should revert commits 7fb7129a832b4f69800eb4b49ed2a99904ed2e7c and 0fa75d404b3bc1f94e654c3d843637f204fa42f2
I'm not sure of which method makes more sense, both logically and regarding performances.
Comment 6 Sebastian Dröge (slomo) 2010-10-25 12:57:16 UTC
Both patches make sense. You should ask upstream or answer it yourself if possible. Unfortunately there's no real policy if you should first ask upstream or should first try to answer the query yourself.

Some elements are doing the former, some the latter...
Comment 7 Sebastian Dröge (slomo) 2010-10-25 12:57:29 UTC
Comment on attachment 172867 [details] [review]
mpeg2dec: convert the position to stream time before answering to a position query

commit ff60a26cf8f66c1a9fc7c8f3b78901f756dd2dfd
Author: Guillaume Emont <gemont@igalia.com>
Date:   Wed Oct 20 20:26:45 2010 +0200

    mpeg2dec: convert the position to stream time before answering to a position query