[FFmpeg-devel] [PATCHv2 4/4] avformat/concatdec: always re-calculate start time and duration

[Marton Balint]

On Fri, 4 Jan 2019, Nicolas George wrote:

> Marton Balint (12018-12-30):
>> This allows the underlying files to change their duration on subsequent
>> avformat context opens.
>>
>> An example use case where this matters:
>>
>> ffconcat version 1.0
>> file dummy.mxf
>> file dummy.mxf
>>
>> ffmpeg -re -stream_loop -1 -i dummy.ffconcat -f sdl2 none
>>
>> The user can seamlessly change the input by atomically replacing dummy.mxf.
>>
>> v2: Set ConcatFile duration in read_header for all segments with known
>> durations because from now on we always recalculate the start time in
>> open_file, and an instant seek could have caused unset ConcatFile durations.
>>
>> Signed-off-by: Marton Balint <cus at passwd.hu>
>> ---
>>  libavformat/concatdec.c | 14 ++++++--------
>>  1 file changed, 6 insertions(+), 8 deletions(-)
>
> I am sorry, but it still feels like a fragile solution, and I think it
> can leave the context in an inconsistent state. Maybe it works now, but
> it is a bad surprise left for somebody who wants to extend this demuxer
> in another way later.
>
> Also, I think the duration detected from the file should never override
> the duration provided by the script.

Agreed, and this is how it should work even after the patch. user_duration 
which is the duration specified in the ffconcat file always have priority 
because get_best_effort_duration always considers that first. So it does 
not matter that we assign ConcatFile->duration again and again, because we 
will just reassing the same value.

Given that, I don't think an inconsitent state is possible because seeking 
is only allowed if the durations are known (if they are all set in 
the ffconcat file). On the other hand, if generic seeking is not allowed, 
then start_time will never be invalid because we always recalculate it for 
the next file when we open it.

Does this relax your concern?

Thanks,
Marton

>
> What about this:
>
> 	if the user_duration is set to a special value then
> 	  each time the file is opened and closed
> 	    if the duration has changed
> 	      update the duration
> 	      update the start time of all subsequent files
>
> ?
>
> The "user_duration is set to a special value" condition also addresses
> another concern I have: this feature conflicts with the (net yet
> implemented) option of keeping the files open, as a LRU-style
> optimization for when much seeking is expected.

Knowing the above
>
> Regards,
>
> --
>  Nicolas George
>