[FFmpeg-devel] [PATCH] mov: Remove ancient heuristic hack

[Michael Niedermayer]

On Wed, Aug 24, 2016 at 03:55:24PM +0100, Derek Buitenhuis wrote:
> This breaks files with legitimate single-entry edit lists,
> and the hack, introduced in f03a081df09f9c4798a17d7e24446ed47924b11b,
> has no link to any known sample in its commit message, nor
> does it actually fix the problem properly, but instead has
> a one-off heuristic to try and "fix" them at the expense
> of breaking legitimate files.
> 
> Signed-off-by: Derek Buitenhuis <derek.buitenhuis at gmail.com>
> ---
> Let's get this out of the way first: I'm not 'returning to FFmpeg'.
> 
> I do not intend to be involved in the community again, nor its
> overall direction and discussions, short of a miracle occurring
> at VideoLAN Dev Days.
> 
> Let's please keep this to technical repleis only. I will ignore
> anything else.
> 
> I am sending this patch in a professional context, as it is
> a problem I have run into at work, with legitimate files,
> produced by x264 and L-SMASH.
> 
> Example file:
>     http://chromashift.org/samples/delay_problem.mp4
> 
> Having the DTS delay output on the first packet itself
> is important for things like cutting files.
> 
> The behavioral change can be seen with:
>     $ ffprobe -show_packets delay_problem.mp4
> 
> Lastly, I would appreciate it if any replies to this patch set
> use 'Reply All', to CC me directly, because I am not currently
> subscribed to this mailing list, and getting proper IDs to use
> in the In-Reply-To field isn't easy now that Gmane is dead.
> 
> Cheers,
> - Derek
> ---
>  libavformat/mov.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 1bc3800..54c63ad 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -2802,12 +2802,8 @@ static void mov_build_index(MOVContext *mov, AVStream *st)
>              sc->time_offset = start_time - empty_duration;
>              current_dts = -sc->time_offset;
>              if (sc->ctts_count>0 && sc->stts_count>0 &&
> -                sc->ctts_data[0].duration / FFMAX(sc->stts_data[0].duration, 1) > 16) {
> -                /* more than 16 frames delay, dts are likely wrong
> -                   this happens with files created by iMovie */
> -                sc->wrong_dts = 1;
> +                sc->ctts_data[0].duration / FFMAX(sc->stts_data[0].duration, 1) > 16)
>                  st->codecpar->video_delay = 1;
> -            }

IIRC the removed code tried to detect a reorder delay that is not
possible in a valid file due to the profile constraints. Aka dts and
pts are too far appart for the largest amount of buffers allowed in
any codec.
Quite possibly this limit or the check itself have become wrong
over time ...
Its even possible there has been some misunderstanding in the buffer
cts/pts/dts limitations.

patch probably ok
iam a bit unsure about "st->codecpar->video_delay = 1;" though


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

What does censorship reveal? It reveals fear. -- Julian Assange
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160824/b68a5d99/attachment.sig>