[FFmpeg-devel] [PATCH] MPEG-TS/-PS : better probing for duration and start-time for all streams in a container

[Michael Niedermayer]

On Thu, Jun 05, 2014 at 04:55:25PM +0000, Gaullier Nicolas wrote:
> >> Please see below my new proposal-patch:
> >> The first_dts is the reason why the start_time was stuck to the first 
> >> packet, since update_initial_timestamps()  returned immediately without any further processing.
> >> I thus skiped this first_dts test.
> >> I then nearly replaced "x=y" with "x=FFMIN(x,y)" with appropriate checkings.
> >> I think it does not change the structure of the code, so it should be safe.
> >> Thank you,
> >> Nicolas
> >
> >patch breaks "make fate"
> 
> (I finally managed to have fate working on my environment, so I will not bother you with patch breaks anymore.)
> 
> It appears that, in the fate-suite, mpeg2/mpeg2_field_encoding.ts is somewhat broken.
> This sample was certainly extracted by means of a simple binary cut. It is common practice with TS, 
> so there is nothing really "wrong" with it, but the fact is that the first broken packets were
> considered valid in the current implementation.
> Besides, since there is no "B" frames (only I/P in this stream), the following fallback rule apply:
> [in utils.c/update_initial_durations]
> if (!st->codec->has_b_frames)
>                 pktl->pkt.pts = cur_dts;
> At the end, a corrupted (truncated) packet is assigned a valid PTS value, and the new algorithm
> of the patch takes it into account, leading to a wrong estimation of the start_time (and fate breaks).
> To resolve this, I think the cleanest solution is to consider the first truncated packets in the stream
> as non-valid. I thus propose here a patch in the avcodec/mpeg layer to impose detection of a picture_start_code.
> That way, it is not possible to assign a PTS where there is no picture.

this wont work, mpeg1/2 in mpeg-ps/ts is not the only case that can
be truncated
and parsers should not discard data, at least not unless the users
tells the parser by some means to discard data

also keep in mind, the fate checksum can be updated if the new result
makes more sense than the old.
I really dont know what the last timestamp should be for a file
with a truncated frame at the end.


> 
> Second thing, there was another break in FATE, because of the CAVS sample (cavs.mpg).
> It appears that in this sample, most of the PES come with no PTS/DTS. I am not aware of CAVS
> and suppose it is normal, but at this end, this must be taken into account in avutils.c/compute_pkt_fields.
> I discovered there was already a onein_oneout defined, so I just appended CAVS.
> There is another onein_oneout in 'select_from_pts_buffer' where CAVS could possibly be appended too,
> but I really do not know anything about CAVS, so I just made the minimum change to have FATE pass.

the mpeg specs dont require PTS/DTS on all PES packets,
onein_oneout is about the relation betwee tne DTS and PTS, not if some
are not coded.
what onein_oneout basically means is that when a decoder is feeded with
a packet at its DTS the decoder outputs and presents at the same
instant a single frame. This also implies if you sort PTS they are
equal to the DTS. I do not know if this is true for CAVS or not.
So i cant say if this change is correct, but it disables PTS/DTS
interpolation for cavs which isnt good unless it has to be done


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Concerning the gods, I have no means of knowing whether they exist or not
or of what sort they may be, because of the obscurity of the subject, and
the brevity of human life -- Protagoras
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140605/f59ce384/attachment.asc>