Re: [FFmpeg-devel] [PATCHv2 4/4] avformat/concatdec: always re-calculate start time and duration
On Mon, 21 Jan 2019, Nicolas George wrote: Marton Balint (12019-01-04): 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? I think it does. Sorry for forgetting to reply to this. Please go ahead when convenient. Thanks, pushed the remaining parts of the series. Regards, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCHv2 4/4] avformat/concatdec: always re-calculate start time and duration
Marton Balint (12019-01-04): > 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? I think it does. Sorry for forgetting to reply to this. Please go ahead when convenient. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCHv2 4/4] avformat/concatdec: always re-calculate start time and duration
On Fri, 4 Jan 2019, Marton Balint wrote: 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 --- 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? Ping. Thanks, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCHv2 4/4] avformat/concatdec: always re-calculate start time and duration
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 --- 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 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCHv2 4/4] avformat/concatdec: always re-calculate start time and duration
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 > --- > 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. 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. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCHv2 4/4] avformat/concatdec: always re-calculate start time and duration
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
---
libavformat/concatdec.c | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c
index 890a220a89..d65da553e1 100644
--- a/libavformat/concatdec.c
+++ b/libavformat/concatdec.c
@@ -355,14 +355,12 @@ static int open_file(AVFormatContext *avf, unsigned
fileno)
return ret;
}
cat->cur_file = file;
-if (file->start_time == AV_NOPTS_VALUE)
-file->start_time = !fileno ? 0 :
- cat->files[fileno - 1].start_time +
- cat->files[fileno - 1].duration;
+file->start_time = !fileno ? 0 :
+ cat->files[fileno - 1].start_time +
+ cat->files[fileno - 1].duration;
file->file_start_time = (cat->avf->start_time == AV_NOPTS_VALUE) ? 0 :
cat->avf->start_time;
file->file_inpoint = (file->inpoint == AV_NOPTS_VALUE) ?
file->file_start_time : file->inpoint;
-if (file->duration == AV_NOPTS_VALUE)
-file->duration = get_best_effort_duration(file, cat->avf);
+file->duration = get_best_effort_duration(file, cat->avf);
if (cat->segment_time_metadata) {
av_dict_set_int(&file->metadata, "lavf.concatdec.start_time",
file->start_time, 0);
@@ -504,6 +502,7 @@ static int concat_read_header(AVFormatContext *avf)
break;
cat->files[i].user_duration = cat->files[i].outpoint -
cat->files[i].inpoint;
}
+cat->files[i].duration = cat->files[i].user_duration;
time += cat->files[i].user_duration;
}
if (i == cat->nb_files) {
@@ -529,8 +528,7 @@ static int open_next_file(AVFormatContext *avf)
ConcatContext *cat = avf->priv_data;
unsigned fileno = cat->cur_file - cat->files;
-if (cat->cur_file->duration == AV_NOPTS_VALUE)
-cat->cur_file->duration = get_best_effort_duration(cat->cur_file,
cat->avf);
+cat->cur_file->duration = get_best_effort_duration(cat->cur_file,
cat->avf);
if (++fileno >= cat->nb_files) {
cat->eof = 1;
--
2.16.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
