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 708532 - tsdemux: skips too much when scanning for last PCR
tsdemux: skips too much when scanning for last PCR
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other All
: Normal normal
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-09-21 13:43 UTC by Matej Knopp
Modified: 2014-12-01 14:06 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch (859 bytes, patch)
2013-09-21 13:43 UTC, Matej Knopp
rejected Details | Review
mpegtbase: Improve last PCR detection (4.48 KB, patch)
2014-07-09 08:17 UTC, Edward Hervey
committed Details | Review

Description Matej Knopp 2013-09-21 13:43:08 UTC
Created attachment 255480 [details] [review]
Patch

Probably a typo. Causes shorter duration reported than the real duration.
Comment 1 Matej Knopp 2013-09-21 13:44:23 UTC
I'm also not sure about this:
    if (mpegts_packetizer_has_packets (base->packetizer)) {
      while (1) {
        /* Eat up all packets */
        pret = mpegts_packetizer_process_next_packet (base->packetizer);
        if (pret == PACKET_NEED_MORE)
          break;
        if (pret != PACKET_BAD &&
            mpegts_packetizer_get_seen_pcr (base->packetizer) >
            initial_pcr_seen) {
          GST_DEBUG ("Got last PCR");
          done = TRUE;
>>        break;
        }
      }
    }

Why is the break there? This will find the first PCR in the block, but that is not necessarily the last PCR in file.
Comment 2 Edward Hervey 2013-09-28 11:18:39 UTC
I pushed a refactoring of pcr/offset handling. Please try again, the results are quite different now.
Comment 3 Edward Hervey 2013-09-28 11:37:58 UTC
The reason why it breaks is to limit amount of data to scan (and get results faster). With the new pcr/offset handling it ends up giving a proper duration.
Comment 4 Edward Hervey 2014-07-09 08:17:52 UTC
Created attachment 280211 [details] [review]
mpegtbase: Improve last PCR detection

When dealing with random-access content (such as files), we initially
search for the last PCR in order to figure out duration and to handle
other position estimation such as those used in seeking.

Previously, the code looking for that last PCR would search in the last
640kB of the file going forward, and stop at the first PCR encountered.
The problem with that was two-fold:
* It wouldn't really be the last PCR (it would be the first one within
  those last 640kB. In case of VBR files, this would put off duration
  and seek code slightly.
* It would fail on files with bitrates higher than 52Mbit/s (not common)

Instead this patch modifies that code by:
* Scanning over the last 2048kB (allows to cope with streams up to 160Mbit/s)
* Starts by the end of the file, going over chunks of 300 MPEG-TS packets
* Doesn't stop at the first PCR detected in a chunk, but instead records all
  of them, and only stop searching if there was "at least" one PCR within
  that chunk

This should improve duration reporting and seeking operations on VBR files
Comment 5 Edward Hervey 2014-12-01 14:05:45 UTC
commit 56ae254dd31f6c1ca468aa43bb3262d2fc84422e
Author: Edward Hervey <bilboed@bilboed.com>
Date:   Wed Jul 9 10:11:40 2014 +0200

    mpegtbase: Improve last PCR detection
    
    When dealing with random-access content (such as files), we initially
    search for the last PCR in order to figure out duration and to handle
    other position estimation such as those used in seeking.
    
    Previously, the code looking for that last PCR would search in the last
    640kB of the file going forward, and stop at the first PCR encountered.
    The problem with that was two-fold:
    * It wouldn't really be the last PCR (it would be the first one within
      those last 640kB. In case of VBR files, this would put off duration
      and seek code slightly.
    * It would fail on files with bitrates higher than 52Mbit/s (not common)
    
    Instead this patch modifies that code by:
    * Scanning over the last 2048kB (allows to cope with streams up to 160Mbit/s)
    * Starts by the end of the file, going over chunks of 300 MPEG-TS packets
    * Doesn't stop at the first PCR detected in a chunk, but instead records all
      of them, and only stop searching if there was "at least" one PCR within
      that chunk
    
    This should improve duration reporting and seeking operations on VBR files
    
    https://bugzilla.gnome.org/show_bug.cgi?id=708532