Thanks Andreas On Wed, Mar 11, 2020 at 5:35 PM Andreas Rheinhardt < andreas.rheinha...@gmail.com> wrote:
> Thierry Foucu: > > On Wed, Mar 11, 2020 at 10:13 AM Andreas Rheinhardt < > > andreas.rheinha...@gmail.com> wrote: > > > >> Thierry Foucu: > >>> --- > >>> libavformat/matroskaenc.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c > >>> index 42f21eae8b..cf436f9e3e 100644 > >>> --- a/libavformat/matroskaenc.c > >>> +++ b/libavformat/matroskaenc.c > >>> @@ -2630,7 +2630,7 @@ static int mkv_write_trailer(AVFormatContext *s) > >>> avio_seek(pb, currentpos, SEEK_SET); > >>> } > >>> > >>> - if (!mkv->is_live) { > >>> + if ((pb->seekable & AVIO_SEEKABLE_NORMAL) && !mkv->is_live) { > >>> end_ebml_master(pb, mkv->segment); > >>> } > >>> > >>> > >> Why? Even if the output is unseekable, the seek to the beginning might > >> succeed if it happens in the output buffer (unlikely but possible). Or > >> do you have a specific usecase in mind where the segment should be > >> unknown-length? > >> > >> > > In most cases in the muxer, we are checking if the file support seeking > > before writing some ebml, like we do for the cues in the trailer > function. > > It is true that in some cases, the offset will be in the output buffer, > > then why not as well check it for cues? > > Because in order to write cues, one would have to store them first and > we don't do that given that it is very unlikely for the cues to be > used later. > Furthermore, in the other cases (e.g. the Info element): These are > already written and freed when writing the header if the output is > unseekable (or actually: if the output was unseekable during writing > the header; the logic is wrong if the output's seekability status > changes and dangerous if it was unseekable when writing the header and > changes to seekable later; I'll send a patch for this). > > > but most of the time, this offset will not when we are in streaming > more. > > > > Maybe the aviobuf layer should not then call io_seek if the under > > layer has is_streamed > > = true? > > > First of all, the aviobuf layer doesn't call io_seek() any more > because it has been removed in 6e8e8431. And does it matter from which > layer the error for a failed seek comes if the seek was unsuccessfull? > (Furthermore, there is (at least?) an instance where the is_streamed > is wrong: Namely if you use the seekable option for file output. But > that seems to be intentional.) > > We have some code to check if ffmpeg was calling seek for some streamable url or not and found this one. what about this: not only check for the seek function to be present (which will true for file.c) but check if it is seekable? =================================== diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c index cc22beff28..bbf7545750 100644 --- a/libavformat/aviobuf.c +++ b/libavformat/aviobuf.c @@ -312,7 +312,7 @@ int64_t avio_seek(AVIOContext *s, int64_t offset, int whence) if (s->write_flag) { flush_buffer(s); } - if (!s->seek) + if (!s->seek || s->seekable == 0) return AVERROR(EPIPE); if ((res = s->seek(s->opaque, offset, SEEK_SET)) < 0) return res; > - Andreas > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". -- Thierry Foucu _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".