Derek Buitenhuis:
> When doing streamed output, with e.g. +dash, if the mfra box ended
> up being larger than the AVIOContext write buffer, the (unchecked)
> seeking back to update the box size would silently fail and produce
> an invalid mfra box.
>
> This is similar to how other boxes are written in fragmented mode.
>
> Signed-off-by: Derek Buitenhuis <derek.buitenh...@gmail.com>
> ---
> libavformat/movenc.c | 40 ++++++++++++++++++++++++++++------------
> 1 file changed, 28 insertions(+), 12 deletions(-)
>
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index b8e45760ee..2927865cb4 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -4798,28 +4798,42 @@ static int mov_write_tfra_tag(AVIOContext *pb,
> MOVTrack *track)
>
> static int mov_write_mfra_tag(AVIOContext *pb, MOVMuxContext *mov)
> {
> - int64_t pos = avio_tell(pb);
> - int i;
> + AVIOContext *mfra_pb;
> + int i, ret, sz;
> + uint8_t *buf;
>
> - avio_wb32(pb, 0); /* size placeholder */
> - ffio_wfourcc(pb, "mfra");
> + ret = avio_open_dyn_buf(&mfra_pb);
> + if (ret < 0)
> + return ret;
> +
> + avio_wb32(mfra_pb, 0); /* size placeholder */
> + ffio_wfourcc(mfra_pb, "mfra");
> /* An empty mfra atom is enough to indicate to the publishing point that
> * the stream has ended. */
> if (mov->flags & FF_MOV_FLAG_ISML)
> - return update_size(pb, pos);
> + goto done_mfra;
>
> for (i = 0; i < mov->nb_streams; i++) {
> MOVTrack *track = &mov->tracks[i];
> if (track->nb_frag_info)
> - mov_write_tfra_tag(pb, track);
> + mov_write_tfra_tag(mfra_pb, track);
> }
>
> - avio_wb32(pb, 16);
> - ffio_wfourcc(pb, "mfro");
> - avio_wb32(pb, 0); /* version + flags */
> - avio_wb32(pb, avio_tell(pb) + 4 - pos);
> + avio_wb32(mfra_pb, 16);
> + ffio_wfourcc(mfra_pb, "mfro");
> + avio_wb32(mfra_pb, 0); /* version + flags */
> + avio_wb32(mfra_pb, avio_tell(mfra_pb) + 4);
>
> - return update_size(pb, pos);
> +done_mfra:
> +
> + sz = update_size(mfra_pb, 0);
> + ret = avio_close_dyn_buf(mfra_pb, &buf);
Use avio_get_dyn_buf in combination with ffio_free_dyn_buf. That way you
save an allocation+copy in case the data fits into the dynamic buffer's
write buffer (it has a size of 1024 bytes; I don't know how common it is
for this box to be smaller).
> + if (ret < 0)
> + return ret;
Furthermore, avio_close_dyn_buf doesn't even provide a way to check for
errors; e.g. it does not allow you to distinguish between truncation
errors and non-truncation errors (due to returning an int, the internal
buffer is capped to INT_MAX). avio_close_dyn_buf actually may not return
anything negative (it is documented to just return the length of the
byte buffer), yet due to a bug it returns -AV_INPUT_BUFFER_PADDING_SIZE
on allocation error.
In contrast there is a way to check for errors with avio_get_dyn_buf:
Check the AVIOContext's error flag after avio_get_dyn_buf.
> + avio_write(pb, buf, ret);
> + av_free(buf);
> +
> + return sz;
> }
>
> static int mov_write_mdat_tag(AVIOContext *pb, MOVMuxContext *mov)
> @@ -6987,7 +7001,9 @@ static int mov_write_trailer(AVFormatContext *s)
> }
> if (!(mov->flags & FF_MOV_FLAG_SKIP_TRAILER)) {
> avio_write_marker(s->pb, AV_NOPTS_VALUE,
> AVIO_DATA_MARKER_TRAILER);
> - mov_write_mfra_tag(pb, mov);
> + res = mov_write_mfra_tag(pb, mov);
> + if (res < 0)
> + return res;
> }
> }
>
>
_______________________________________________
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".
