[FFmpeg-devel] [PATCH 1/2] avformat/matroskaenc: fix Tags master on seekable output if there are tags after the last stream duration

[James Almer]

On 10/7/2016 3:29 PM, Dave Rice wrote:
> 
>> On Oct 7, 2016, at 10:30 AM, James Almer <jamrial at gmail.com> wrote:
>>
>> On 10/7/2016 3:05 AM, James Almer wrote:
>>> The dynamic AVIOContext would get closed pointing to the wrong position
>>> in the buffer.
>>> This is a regression since 650e17d88b63b5aca6e0a43483e89e64b0f7d2dd.
>>>
>>> Signed-off-by: James Almer <jamrial at gmail.com>
>>> ---
>>> Example:
>>> ./ffmpeg -i fate-samples/vorbis/vorbis_chapter_extension_demo.ogg -c:a copy -metadata:c key=value out.mka
>>>
>>> The output is corrupt before this patch.
>>>
>>> The refactoring isn't necessary per se, but it comes in handy for the next patch,
>>> where attachments will not be treated as tracks anymore when writting tags.
>>>
>>> libavformat/matroskaenc.c | 15 +++++++++------
>>> 1 file changed, 9 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
>>> index 0878cb5..286b8b4 100644
>>> --- a/libavformat/matroskaenc.c
>>> +++ b/libavformat/matroskaenc.c
>>> @@ -2264,17 +2264,19 @@ static int mkv_write_trailer(AVFormatContext *s)
>>>         end_ebml_master_crc32(pb, &mkv->info_bc, mkv, mkv->info);
>>>
>>>         // update stream durations
>>> -        if (mkv->stream_durations) {
>>> +        if (!mkv->is_live && mkv->stream_durations) {
>>>             int i;
>>> +            int64_t curr = avio_tell(mkv->tags_bc);
>>>             for (i = 0; i < s->nb_streams; ++i) {
>>>                 AVStream *st = s->streams[i];
>>> -                double duration_sec = mkv->stream_durations[i] * av_q2d(st->time_base);
>>> -                char duration_string[20] = "";
>>>
>>> -                av_log(s, AV_LOG_DEBUG, "stream %d end duration = %" PRIu64 "\n", i,
>>> -                       mkv->stream_durations[i]);
>>> +                if (mkv->stream_duration_offsets[i] > 0) {
>>> +                    double duration_sec = mkv->stream_durations[i] * av_q2d(st->time_base);
>>> +                    char duration_string[20] = "";
>>> +
>>> +                    av_log(s, AV_LOG_DEBUG, "stream %d end duration = %" PRIu64 "\n", i,
>>> +                           mkv->stream_durations[i]);
>>>
>>> -                if (!mkv->is_live && mkv->stream_duration_offsets[i] > 0) {
>>>                     avio_seek(mkv->tags_bc, mkv->stream_duration_offsets[i], SEEK_SET);
>>>
>>>                     snprintf(duration_string, 20, "%02d:%02d:%012.9f",
>>> @@ -2284,6 +2286,7 @@ static int mkv_write_trailer(AVFormatContext *s)
>>>                     put_ebml_binary(mkv->tags_bc, MATROSKA_ID_TAGSTRING, duration_string, 20);
>>>                 }
>>>             }
>>> +            avio_seek(mkv->tags_bc, curr, SEEK_SET);
>>>         }
>>>         if (mkv->tags.pos && !mkv->is_live) {
>>>             avio_seek(pb, mkv->tags.pos, SEEK_SET);
>>
>>
>> Ping. It's a recent regression on a muxer, so I'd like to solve it asap.
>> I'll push it later today if nobody comments.
> 
> I tested this and it does fix the issue so that the added "key" tag is properly formed, whereas the tag is only null bytes without the patch. Thanks!
> Dave Rice

Pushed, thanks.