| Message ID | 20210204191005.48190-22-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | deprecate av_init_packet() and sizeof(AVPacket) as part of the ABI | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
James Almer: > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavformat/matroskadec.c | 17 ++++++++++++----- > 1 file changed, 12 insertions(+), 5 deletions(-) > > diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c > index 374831baa3..9fad78c78b 100644 > --- a/libavformat/matroskadec.c > +++ b/libavformat/matroskadec.c > @@ -365,6 +365,8 @@ typedef struct MatroskaDemuxContext { > /* byte position of the segment inside the stream */ > int64_t segment_start; > > + AVPacket *pkt; > + > /* the packet queue */ > AVPacketList *queue; > AVPacketList *queue_end; > @@ -2885,6 +2887,10 @@ static int matroska_read_header(AVFormatContext *s) > } > ebml_free(ebml_syntax, &ebml); > > + matroska->pkt = av_packet_alloc(); > + if (!matroska->pkt) Missing AVERROR(ENOMEM). (I actually have an idea to completely remove the packet list from matroskadec.) > + goto fail; > + > /* The next thing is a segment. */ > pos = avio_tell(matroska->ctx->pb); > res = ebml_parse(matroska, matroska_segments, matroska); > @@ -2947,7 +2953,7 @@ static int matroska_read_header(AVFormatContext *s) > st->disposition |= AV_DISPOSITION_ATTACHED_PIC; > st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO; > > - av_init_packet(pkt); > + av_packet_unref(pkt); > pkt->buf = attachments[j].bin.buf; > attachments[j].bin.buf = NULL; > pkt->data = attachments[j].bin.data; > @@ -3180,7 +3186,7 @@ static int matroska_parse_rm_audio(MatroskaDemuxContext *matroska, > > while (track->audio.pkt_cnt) { > int ret; > - AVPacket pktl, *pkt = &pktl; > + AVPacket *pkt = matroska->pkt; > > ret = av_new_packet(pkt, a); > if (ret < 0) { > @@ -3317,7 +3323,7 @@ static int matroska_parse_webvtt(MatroskaDemuxContext *matroska, > uint64_t duration, > int64_t pos) > { > - AVPacket pktl, *pkt = &pktl; > + AVPacket *pkt = matroska->pkt; > uint8_t *id, *settings, *text, *buf; > int id_len, settings_len, text_len; > uint8_t *p, *q; > @@ -3434,7 +3440,7 @@ static int matroska_parse_frame(MatroskaDemuxContext *matroska, > { > uint8_t *pkt_data = data; > int res = 0; > - AVPacket pktl, *pkt = &pktl; > + AVPacket *pkt = matroska->pkt; > > if (st->codecpar->codec_id == AV_CODEC_ID_WAVPACK) { > res = matroska_parse_wavpack(track, &pkt_data, &pkt_size); > @@ -3464,7 +3470,7 @@ static int matroska_parse_frame(MatroskaDemuxContext *matroska, > if (!pkt_size && !additional_size) > goto no_output; > > - av_init_packet(pkt); > + av_packet_unref(pkt); > if (!buf) > pkt->buf = av_buffer_create(pkt_data, pkt_size + AV_INPUT_BUFFER_PADDING_SIZE, > NULL, NULL, 0); > @@ -3838,6 +3844,7 @@ static int matroska_read_close(AVFormatContext *s) > int n; > > matroska_clear_queue(matroska); > + av_packet_free(&matroska->pkt); > > for (n = 0; n < matroska->tracks.nb_elem; n++) > if (tracks[n].type == MATROSKA_TRACK_TYPE_AUDIO) >
On 2/8/2021 12:22 PM, Andreas Rheinhardt wrote: > James Almer: >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavformat/matroskadec.c | 17 ++++++++++++----- >> 1 file changed, 12 insertions(+), 5 deletions(-) >> >> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c >> index 374831baa3..9fad78c78b 100644 >> --- a/libavformat/matroskadec.c >> +++ b/libavformat/matroskadec.c >> @@ -365,6 +365,8 @@ typedef struct MatroskaDemuxContext { >> /* byte position of the segment inside the stream */ >> int64_t segment_start; >> >> + AVPacket *pkt; >> + >> /* the packet queue */ >> AVPacketList *queue; >> AVPacketList *queue_end; >> @@ -2885,6 +2887,10 @@ static int matroska_read_header(AVFormatContext *s) >> } >> ebml_free(ebml_syntax, &ebml); >> >> + matroska->pkt = av_packet_alloc(); >> + if (!matroska->pkt) > > Missing AVERROR(ENOMEM). This seems to be a common mistake in this set. Sorry. > (I actually have an idea to completely remove the packet list from > matroskadec.) Should i commit this while you work on that, or just withdraw this patch? It's not urgent at all, since any deprecation period is 2+ years. Either option is fine by me. > >> + goto fail; >> + >> /* The next thing is a segment. */ >> pos = avio_tell(matroska->ctx->pb); >> res = ebml_parse(matroska, matroska_segments, matroska); >> @@ -2947,7 +2953,7 @@ static int matroska_read_header(AVFormatContext *s) >> st->disposition |= AV_DISPOSITION_ATTACHED_PIC; >> st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO; >> >> - av_init_packet(pkt); >> + av_packet_unref(pkt); >> pkt->buf = attachments[j].bin.buf; >> attachments[j].bin.buf = NULL; >> pkt->data = attachments[j].bin.data; >> @@ -3180,7 +3186,7 @@ static int matroska_parse_rm_audio(MatroskaDemuxContext *matroska, >> >> while (track->audio.pkt_cnt) { >> int ret; >> - AVPacket pktl, *pkt = &pktl; >> + AVPacket *pkt = matroska->pkt; >> >> ret = av_new_packet(pkt, a); >> if (ret < 0) { >> @@ -3317,7 +3323,7 @@ static int matroska_parse_webvtt(MatroskaDemuxContext *matroska, >> uint64_t duration, >> int64_t pos) >> { >> - AVPacket pktl, *pkt = &pktl; >> + AVPacket *pkt = matroska->pkt; >> uint8_t *id, *settings, *text, *buf; >> int id_len, settings_len, text_len; >> uint8_t *p, *q; >> @@ -3434,7 +3440,7 @@ static int matroska_parse_frame(MatroskaDemuxContext *matroska, >> { >> uint8_t *pkt_data = data; >> int res = 0; >> - AVPacket pktl, *pkt = &pktl; >> + AVPacket *pkt = matroska->pkt; >> >> if (st->codecpar->codec_id == AV_CODEC_ID_WAVPACK) { >> res = matroska_parse_wavpack(track, &pkt_data, &pkt_size); >> @@ -3464,7 +3470,7 @@ static int matroska_parse_frame(MatroskaDemuxContext *matroska, >> if (!pkt_size && !additional_size) >> goto no_output; >> >> - av_init_packet(pkt); >> + av_packet_unref(pkt); >> if (!buf) >> pkt->buf = av_buffer_create(pkt_data, pkt_size + AV_INPUT_BUFFER_PADDING_SIZE, >> NULL, NULL, 0); >> @@ -3838,6 +3844,7 @@ static int matroska_read_close(AVFormatContext *s) >> int n; >> >> matroska_clear_queue(matroska); >> + av_packet_free(&matroska->pkt); >> >> for (n = 0; n < matroska->tracks.nb_elem; n++) >> if (tracks[n].type == MATROSKA_TRACK_TYPE_AUDIO) >> > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
James Almer: > On 2/8/2021 12:22 PM, Andreas Rheinhardt wrote: >> James Almer: >>> Signed-off-by: James Almer <jamrial@gmail.com> >>> --- >>> libavformat/matroskadec.c | 17 ++++++++++++----- >>> 1 file changed, 12 insertions(+), 5 deletions(-) >>> >>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c >>> index 374831baa3..9fad78c78b 100644 >>> --- a/libavformat/matroskadec.c >>> +++ b/libavformat/matroskadec.c >>> @@ -365,6 +365,8 @@ typedef struct MatroskaDemuxContext { >>> /* byte position of the segment inside the stream */ >>> int64_t segment_start; >>> + AVPacket *pkt; >>> + >>> /* the packet queue */ >>> AVPacketList *queue; >>> AVPacketList *queue_end; >>> @@ -2885,6 +2887,10 @@ static int >>> matroska_read_header(AVFormatContext *s) >>> } >>> ebml_free(ebml_syntax, &ebml); >>> + matroska->pkt = av_packet_alloc(); >>> + if (!matroska->pkt) >> >> Missing AVERROR(ENOMEM). > > This seems to be a common mistake in this set. Sorry. > Yeah, the failure paths seem untested. >> (I actually have an idea to completely remove the packet list from >> matroskadec.) > > Should i commit this while you work on that, or just withdraw this > patch? It's not urgent at all, since any deprecation period is 2+ years. > Either option is fine by me. > I don't want to have deprecation warnings. >> >>> + goto fail; >>> + >>> /* The next thing is a segment. */ >>> pos = avio_tell(matroska->ctx->pb); >>> res = ebml_parse(matroska, matroska_segments, matroska); >>> @@ -2947,7 +2953,7 @@ static int matroska_read_header(AVFormatContext >>> *s) >>> st->disposition |= >>> AV_DISPOSITION_ATTACHED_PIC; >>> st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO; >>> - av_init_packet(pkt); >>> + av_packet_unref(pkt); >>> pkt->buf = attachments[j].bin.buf; >>> attachments[j].bin.buf = NULL; >>> pkt->data = attachments[j].bin.data; >>> @@ -3180,7 +3186,7 @@ static int >>> matroska_parse_rm_audio(MatroskaDemuxContext *matroska, >>> while (track->audio.pkt_cnt) { >>> int ret; >>> - AVPacket pktl, *pkt = &pktl; >>> + AVPacket *pkt = matroska->pkt; >>> ret = av_new_packet(pkt, a); >>> if (ret < 0) { >>> @@ -3317,7 +3323,7 @@ static int >>> matroska_parse_webvtt(MatroskaDemuxContext *matroska, >>> uint64_t duration, >>> int64_t pos) >>> { >>> - AVPacket pktl, *pkt = &pktl; >>> + AVPacket *pkt = matroska->pkt; >>> uint8_t *id, *settings, *text, *buf; >>> int id_len, settings_len, text_len; >>> uint8_t *p, *q; >>> @@ -3434,7 +3440,7 @@ static int >>> matroska_parse_frame(MatroskaDemuxContext *matroska, >>> { >>> uint8_t *pkt_data = data; >>> int res = 0; >>> - AVPacket pktl, *pkt = &pktl; >>> + AVPacket *pkt = matroska->pkt; >>> if (st->codecpar->codec_id == AV_CODEC_ID_WAVPACK) { >>> res = matroska_parse_wavpack(track, &pkt_data, &pkt_size); >>> @@ -3464,7 +3470,7 @@ static int >>> matroska_parse_frame(MatroskaDemuxContext *matroska, >>> if (!pkt_size && !additional_size) >>> goto no_output; >>> - av_init_packet(pkt); >>> + av_packet_unref(pkt); >>> if (!buf) >>> pkt->buf = av_buffer_create(pkt_data, pkt_size + >>> AV_INPUT_BUFFER_PADDING_SIZE, >>> NULL, NULL, 0); >>> @@ -3838,6 +3844,7 @@ static int matroska_read_close(AVFormatContext *s) >>> int n; >>> matroska_clear_queue(matroska); >>> + av_packet_free(&matroska->pkt); >>> for (n = 0; n < matroska->tracks.nb_elem; n++) >>> if (tracks[n].type == MATROSKA_TRACK_TYPE_AUDIO) >>> >> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >> > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On 2/8/2021 12:32 PM, Andreas Rheinhardt wrote: > James Almer: >> On 2/8/2021 12:22 PM, Andreas Rheinhardt wrote: >>> James Almer: >>>> Signed-off-by: James Almer <jamrial@gmail.com> >>>> --- >>>> libavformat/matroskadec.c | 17 ++++++++++++----- >>>> 1 file changed, 12 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c >>>> index 374831baa3..9fad78c78b 100644 >>>> --- a/libavformat/matroskadec.c >>>> +++ b/libavformat/matroskadec.c >>>> @@ -365,6 +365,8 @@ typedef struct MatroskaDemuxContext { >>>> /* byte position of the segment inside the stream */ >>>> int64_t segment_start; >>>> + AVPacket *pkt; >>>> + >>>> /* the packet queue */ >>>> AVPacketList *queue; >>>> AVPacketList *queue_end; >>>> @@ -2885,6 +2887,10 @@ static int >>>> matroska_read_header(AVFormatContext *s) >>>> } >>>> ebml_free(ebml_syntax, &ebml); >>>> + matroska->pkt = av_packet_alloc(); >>>> + if (!matroska->pkt) >>> >>> Missing AVERROR(ENOMEM). >> >> This seems to be a common mistake in this set. Sorry. >> > > Yeah, the failure paths seem untested. make fate doesn't test failure paths. I recall someone did some manual runs limiting available memory long ago, which detected and removed a lot of unchecked av_mallocs, but nothing automated. Fuzzing has handled failure path issue detection since then. > >>> (I actually have an idea to completely remove the packet list from >>> matroskadec.) >> >> Should i commit this while you work on that, or just withdraw this >> patch? It's not urgent at all, since any deprecation period is 2+ years. >> Either option is fine by me. >> > I don't want to have deprecation warnings. Ok. > >>> >>>> + goto fail; >>>> + >>>> /* The next thing is a segment. */ >>>> pos = avio_tell(matroska->ctx->pb); >>>> res = ebml_parse(matroska, matroska_segments, matroska); >>>> @@ -2947,7 +2953,7 @@ static int matroska_read_header(AVFormatContext >>>> *s) >>>> st->disposition |= >>>> AV_DISPOSITION_ATTACHED_PIC; >>>> st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO; >>>> - av_init_packet(pkt); >>>> + av_packet_unref(pkt); >>>> pkt->buf = attachments[j].bin.buf; >>>> attachments[j].bin.buf = NULL; >>>> pkt->data = attachments[j].bin.data; >>>> @@ -3180,7 +3186,7 @@ static int >>>> matroska_parse_rm_audio(MatroskaDemuxContext *matroska, >>>> while (track->audio.pkt_cnt) { >>>> int ret; >>>> - AVPacket pktl, *pkt = &pktl; >>>> + AVPacket *pkt = matroska->pkt; >>>> ret = av_new_packet(pkt, a); >>>> if (ret < 0) { >>>> @@ -3317,7 +3323,7 @@ static int >>>> matroska_parse_webvtt(MatroskaDemuxContext *matroska, >>>> uint64_t duration, >>>> int64_t pos) >>>> { >>>> - AVPacket pktl, *pkt = &pktl; >>>> + AVPacket *pkt = matroska->pkt; >>>> uint8_t *id, *settings, *text, *buf; >>>> int id_len, settings_len, text_len; >>>> uint8_t *p, *q; >>>> @@ -3434,7 +3440,7 @@ static int >>>> matroska_parse_frame(MatroskaDemuxContext *matroska, >>>> { >>>> uint8_t *pkt_data = data; >>>> int res = 0; >>>> - AVPacket pktl, *pkt = &pktl; >>>> + AVPacket *pkt = matroska->pkt; >>>> if (st->codecpar->codec_id == AV_CODEC_ID_WAVPACK) { >>>> res = matroska_parse_wavpack(track, &pkt_data, &pkt_size); >>>> @@ -3464,7 +3470,7 @@ static int >>>> matroska_parse_frame(MatroskaDemuxContext *matroska, >>>> if (!pkt_size && !additional_size) >>>> goto no_output; >>>> - av_init_packet(pkt); >>>> + av_packet_unref(pkt); >>>> if (!buf) >>>> pkt->buf = av_buffer_create(pkt_data, pkt_size + >>>> AV_INPUT_BUFFER_PADDING_SIZE, >>>> NULL, NULL, 0); >>>> @@ -3838,6 +3844,7 @@ static int matroska_read_close(AVFormatContext *s) >>>> int n; >>>> matroska_clear_queue(matroska); >>>> + av_packet_free(&matroska->pkt); >>>> for (n = 0; n < matroska->tracks.nb_elem; n++) >>>> if (tracks[n].type == MATROSKA_TRACK_TYPE_AUDIO) >>>> >>> >>> _______________________________________________ >>> ffmpeg-devel mailing list >>> ffmpeg-devel@ffmpeg.org >>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>> >>> To unsubscribe, visit link above, or email >>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >>> >> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
James Almer: > On 2/8/2021 12:32 PM, Andreas Rheinhardt wrote: >> James Almer: >>> On 2/8/2021 12:22 PM, Andreas Rheinhardt wrote: >>>> James Almer: >>>>> Signed-off-by: James Almer <jamrial@gmail.com> >>>>> --- >>>>> libavformat/matroskadec.c | 17 ++++++++++++----- >>>>> 1 file changed, 12 insertions(+), 5 deletions(-) >>>>> >>>>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c >>>>> index 374831baa3..9fad78c78b 100644 >>>>> --- a/libavformat/matroskadec.c >>>>> +++ b/libavformat/matroskadec.c >>>>> @@ -365,6 +365,8 @@ typedef struct MatroskaDemuxContext { >>>>> /* byte position of the segment inside the stream */ >>>>> int64_t segment_start; >>>>> + AVPacket *pkt; >>>>> + >>>>> /* the packet queue */ >>>>> AVPacketList *queue; >>>>> AVPacketList *queue_end; >>>>> @@ -2885,6 +2887,10 @@ static int >>>>> matroska_read_header(AVFormatContext *s) >>>>> } >>>>> ebml_free(ebml_syntax, &ebml); >>>>> + matroska->pkt = av_packet_alloc(); >>>>> + if (!matroska->pkt) >>>> >>>> Missing AVERROR(ENOMEM). >>> >>> This seems to be a common mistake in this set. Sorry. >>> >> >> Yeah, the failure paths seem untested. > > make fate doesn't test failure paths. I recall someone did some manual > runs limiting available memory long ago, which detected and removed a > lot of unchecked av_mallocs, but nothing automated. > Fuzzing has handled failure path issue detection since then. > No, fuzzing does not simulate low-memory environments (at least ossfuzz as-is does not). I think it errors out on OOM when one tries to allocate a lot, but small stuff like AVPackets don't trigger this. I have a long list of patches in libavcodec where an earlier allocation in an init function would leak if a subsequent allocation fails; or even something like 96061c5a4f690c3ab49e4458701bb013fd3dd57f where the close function assumed that several allocations were successful. These bugs existed for years and the fuzzer never found them. This means that all these errors paths need to be manually tested, so that something that adds error paths (like requiring things to be allocated) adds a burden on devs (in addition to the runtime overhead). >> >>>> (I actually have an idea to completely remove the packet list from >>>> matroskadec.) >>> >>> Should i commit this while you work on that, or just withdraw this >>> patch? It's not urgent at all, since any deprecation period is 2+ years. >>> Either option is fine by me. >>> >> I don't want to have deprecation warnings. > > Ok. > >> >>>> >>>>> + goto fail; >>>>> + >>>>> /* The next thing is a segment. */ >>>>> pos = avio_tell(matroska->ctx->pb); >>>>> res = ebml_parse(matroska, matroska_segments, matroska); >>>>> @@ -2947,7 +2953,7 @@ static int matroska_read_header(AVFormatContext >>>>> *s) >>>>> st->disposition |= >>>>> AV_DISPOSITION_ATTACHED_PIC; >>>>> st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO; >>>>> - av_init_packet(pkt); >>>>> + av_packet_unref(pkt); >>>>> pkt->buf = attachments[j].bin.buf; >>>>> attachments[j].bin.buf = NULL; >>>>> pkt->data = attachments[j].bin.data; >>>>> @@ -3180,7 +3186,7 @@ static int >>>>> matroska_parse_rm_audio(MatroskaDemuxContext *matroska, >>>>> while (track->audio.pkt_cnt) { >>>>> int ret; >>>>> - AVPacket pktl, *pkt = &pktl; >>>>> + AVPacket *pkt = matroska->pkt; >>>>> ret = av_new_packet(pkt, a); >>>>> if (ret < 0) { >>>>> @@ -3317,7 +3323,7 @@ static int >>>>> matroska_parse_webvtt(MatroskaDemuxContext *matroska, >>>>> uint64_t duration, >>>>> int64_t pos) >>>>> { >>>>> - AVPacket pktl, *pkt = &pktl; >>>>> + AVPacket *pkt = matroska->pkt; >>>>> uint8_t *id, *settings, *text, *buf; >>>>> int id_len, settings_len, text_len; >>>>> uint8_t *p, *q; >>>>> @@ -3434,7 +3440,7 @@ static int >>>>> matroska_parse_frame(MatroskaDemuxContext *matroska, >>>>> { >>>>> uint8_t *pkt_data = data; >>>>> int res = 0; >>>>> - AVPacket pktl, *pkt = &pktl; >>>>> + AVPacket *pkt = matroska->pkt; >>>>> if (st->codecpar->codec_id == AV_CODEC_ID_WAVPACK) { >>>>> res = matroska_parse_wavpack(track, &pkt_data, &pkt_size); >>>>> @@ -3464,7 +3470,7 @@ static int >>>>> matroska_parse_frame(MatroskaDemuxContext *matroska, >>>>> if (!pkt_size && !additional_size) >>>>> goto no_output; >>>>> - av_init_packet(pkt); >>>>> + av_packet_unref(pkt); >>>>> if (!buf) >>>>> pkt->buf = av_buffer_create(pkt_data, pkt_size + >>>>> AV_INPUT_BUFFER_PADDING_SIZE, >>>>> NULL, NULL, 0); >>>>> @@ -3838,6 +3844,7 @@ static int >>>>> matroska_read_close(AVFormatContext *s) >>>>> int n; >>>>> matroska_clear_queue(matroska); >>>>> + av_packet_free(&matroska->pkt); >>>>> for (n = 0; n < matroska->tracks.nb_elem; n++) >>>>> if (tracks[n].type == MATROSKA_TRACK_TYPE_AUDIO) >>>>> >>>>
On 2/8/2021 12:57 PM, Andreas Rheinhardt wrote: > James Almer: >> On 2/8/2021 12:32 PM, Andreas Rheinhardt wrote: >>> James Almer: >>>> On 2/8/2021 12:22 PM, Andreas Rheinhardt wrote: >>>>> James Almer: >>>>>> Signed-off-by: James Almer <jamrial@gmail.com> >>>>>> --- >>>>>> libavformat/matroskadec.c | 17 ++++++++++++----- >>>>>> 1 file changed, 12 insertions(+), 5 deletions(-) >>>>>> >>>>>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c >>>>>> index 374831baa3..9fad78c78b 100644 >>>>>> --- a/libavformat/matroskadec.c >>>>>> +++ b/libavformat/matroskadec.c >>>>>> @@ -365,6 +365,8 @@ typedef struct MatroskaDemuxContext { >>>>>> /* byte position of the segment inside the stream */ >>>>>> int64_t segment_start; >>>>>> + AVPacket *pkt; >>>>>> + >>>>>> /* the packet queue */ >>>>>> AVPacketList *queue; >>>>>> AVPacketList *queue_end; >>>>>> @@ -2885,6 +2887,10 @@ static int >>>>>> matroska_read_header(AVFormatContext *s) >>>>>> } >>>>>> ebml_free(ebml_syntax, &ebml); >>>>>> + matroska->pkt = av_packet_alloc(); >>>>>> + if (!matroska->pkt) >>>>> >>>>> Missing AVERROR(ENOMEM). >>>> >>>> This seems to be a common mistake in this set. Sorry. >>>> >>> >>> Yeah, the failure paths seem untested. >> >> make fate doesn't test failure paths. I recall someone did some manual >> runs limiting available memory long ago, which detected and removed a >> lot of unchecked av_mallocs, but nothing automated. >> Fuzzing has handled failure path issue detection since then. >> > > No, fuzzing does not simulate low-memory environments (at least ossfuzz > as-is does not). I know, which is why i said failure path issues, not OOM specifically. Most failure paths it triggers are due to invalid data. But outside of fuzzing, nothing tests any kind of failure path (Maybe static analyzers?). All FATE tests do is ensure working scenarios keep working. > I think it errors out on OOM when one tries to allocate > a lot, but small stuff like AVPackets don't trigger this. I have a long > list of patches in libavcodec where an earlier allocation in an init > function would leak if a subsequent allocation fails; or even something > like 96061c5a4f690c3ab49e4458701bb013fd3dd57f where the close function > assumed that several allocations were successful. These bugs existed for > years and the fuzzer never found them. > This means that all these errors paths need to be manually tested, so > that something that adds error paths (like requiring things to be > allocated) adds a burden on devs (in addition to the runtime overhead). > >>> >>>>> (I actually have an idea to completely remove the packet list from >>>>> matroskadec.) >>>> >>>> Should i commit this while you work on that, or just withdraw this >>>> patch? It's not urgent at all, since any deprecation period is 2+ years. >>>> Either option is fine by me. >>>> >>> I don't want to have deprecation warnings. >> >> Ok. >> >>> >>>>> >>>>>> + goto fail; >>>>>> + >>>>>> /* The next thing is a segment. */ >>>>>> pos = avio_tell(matroska->ctx->pb); >>>>>> res = ebml_parse(matroska, matroska_segments, matroska); >>>>>> @@ -2947,7 +2953,7 @@ static int matroska_read_header(AVFormatContext >>>>>> *s) >>>>>> st->disposition |= >>>>>> AV_DISPOSITION_ATTACHED_PIC; >>>>>> st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO; >>>>>> - av_init_packet(pkt); >>>>>> + av_packet_unref(pkt); >>>>>> pkt->buf = attachments[j].bin.buf; >>>>>> attachments[j].bin.buf = NULL; >>>>>> pkt->data = attachments[j].bin.data; >>>>>> @@ -3180,7 +3186,7 @@ static int >>>>>> matroska_parse_rm_audio(MatroskaDemuxContext *matroska, >>>>>> while (track->audio.pkt_cnt) { >>>>>> int ret; >>>>>> - AVPacket pktl, *pkt = &pktl; >>>>>> + AVPacket *pkt = matroska->pkt; >>>>>> ret = av_new_packet(pkt, a); >>>>>> if (ret < 0) { >>>>>> @@ -3317,7 +3323,7 @@ static int >>>>>> matroska_parse_webvtt(MatroskaDemuxContext *matroska, >>>>>> uint64_t duration, >>>>>> int64_t pos) >>>>>> { >>>>>> - AVPacket pktl, *pkt = &pktl; >>>>>> + AVPacket *pkt = matroska->pkt; >>>>>> uint8_t *id, *settings, *text, *buf; >>>>>> int id_len, settings_len, text_len; >>>>>> uint8_t *p, *q; >>>>>> @@ -3434,7 +3440,7 @@ static int >>>>>> matroska_parse_frame(MatroskaDemuxContext *matroska, >>>>>> { >>>>>> uint8_t *pkt_data = data; >>>>>> int res = 0; >>>>>> - AVPacket pktl, *pkt = &pktl; >>>>>> + AVPacket *pkt = matroska->pkt; >>>>>> if (st->codecpar->codec_id == AV_CODEC_ID_WAVPACK) { >>>>>> res = matroska_parse_wavpack(track, &pkt_data, &pkt_size); >>>>>> @@ -3464,7 +3470,7 @@ static int >>>>>> matroska_parse_frame(MatroskaDemuxContext *matroska, >>>>>> if (!pkt_size && !additional_size) >>>>>> goto no_output; >>>>>> - av_init_packet(pkt); >>>>>> + av_packet_unref(pkt); >>>>>> if (!buf) >>>>>> pkt->buf = av_buffer_create(pkt_data, pkt_size + >>>>>> AV_INPUT_BUFFER_PADDING_SIZE, >>>>>> NULL, NULL, 0); >>>>>> @@ -3838,6 +3844,7 @@ static int >>>>>> matroska_read_close(AVFormatContext *s) >>>>>> int n; >>>>>> matroska_clear_queue(matroska); >>>>>> + av_packet_free(&matroska->pkt); >>>>>> for (n = 0; n < matroska->tracks.nb_elem; n++) >>>>>> if (tracks[n].type == MATROSKA_TRACK_TYPE_AUDIO) >>>>>> >>>>> > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
On 2/8/2021 12:22 PM, Andreas Rheinhardt wrote: > James Almer: >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavformat/matroskadec.c | 17 ++++++++++++----- >> 1 file changed, 12 insertions(+), 5 deletions(-) >> >> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c >> index 374831baa3..9fad78c78b 100644 >> --- a/libavformat/matroskadec.c >> +++ b/libavformat/matroskadec.c >> @@ -365,6 +365,8 @@ typedef struct MatroskaDemuxContext { >> /* byte position of the segment inside the stream */ >> int64_t segment_start; >> >> + AVPacket *pkt; >> + >> /* the packet queue */ >> AVPacketList *queue; >> AVPacketList *queue_end; >> @@ -2885,6 +2887,10 @@ static int matroska_read_header(AVFormatContext *s) >> } >> ebml_free(ebml_syntax, &ebml); >> >> + matroska->pkt = av_packet_alloc(); >> + if (!matroska->pkt) > > Missing AVERROR(ENOMEM). I think at this point i can just return ENOMEM without jumping to fail.
James Almer: > On 2/8/2021 12:57 PM, Andreas Rheinhardt wrote: >> James Almer: >>> On 2/8/2021 12:32 PM, Andreas Rheinhardt wrote: >>>> James Almer: >>>>> On 2/8/2021 12:22 PM, Andreas Rheinhardt wrote: >>>>>> James Almer: >>>>>>> Signed-off-by: James Almer <jamrial@gmail.com> >>>>>>> --- >>>>>>> libavformat/matroskadec.c | 17 ++++++++++++----- >>>>>>> 1 file changed, 12 insertions(+), 5 deletions(-) >>>>>>> >>>>>>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c >>>>>>> index 374831baa3..9fad78c78b 100644 >>>>>>> --- a/libavformat/matroskadec.c >>>>>>> +++ b/libavformat/matroskadec.c >>>>>>> @@ -365,6 +365,8 @@ typedef struct MatroskaDemuxContext { >>>>>>> /* byte position of the segment inside the stream */ >>>>>>> int64_t segment_start; >>>>>>> + AVPacket *pkt; >>>>>>> + >>>>>>> /* the packet queue */ >>>>>>> AVPacketList *queue; >>>>>>> AVPacketList *queue_end; >>>>>>> @@ -2885,6 +2887,10 @@ static int >>>>>>> matroska_read_header(AVFormatContext *s) >>>>>>> } >>>>>>> ebml_free(ebml_syntax, &ebml); >>>>>>> + matroska->pkt = av_packet_alloc(); >>>>>>> + if (!matroska->pkt) >>>>>> >>>>>> Missing AVERROR(ENOMEM). >>>>> >>>>> This seems to be a common mistake in this set. Sorry. >>>>> >>>> >>>> Yeah, the failure paths seem untested. >>> >>> make fate doesn't test failure paths. I recall someone did some manual >>> runs limiting available memory long ago, which detected and removed a >>> lot of unchecked av_mallocs, but nothing automated. >>> Fuzzing has handled failure path issue detection since then. >>> >> >> No, fuzzing does not simulate low-memory environments (at least ossfuzz >> as-is does not). > > I know, which is why i said failure path issues, not OOM specifically. Then I don't get the point of you mentioning fuzzing as handling failure path issues in your reply to my comment about untested allocation error paths. > Most failure paths it triggers are due to invalid data. But outside of > fuzzing, nothing tests any kind of failure path (Maybe static > analyzers?). All FATE tests do is ensure working scenarios keep working. > Static analyzers do, but they typically only find the easy ones. E.g. Coverity would probably have found the memleak in avpriv_dv_init_demux that your patch introduced, but not the memleaks in the dv demuxer (because in the latter case, the packets are still accessible via the still existing DVDemuxContext and it doesn't know that they will not be freed). (And of course it should be mentioned that not all parts of FFmpeg are systematically fuzzed: Encoders, muxers and filters are not.) >> I think it errors out on OOM when one tries to allocate >> a lot, but small stuff like AVPackets don't trigger this. I have a long >> list of patches in libavcodec where an earlier allocation in an init >> function would leak if a subsequent allocation fails; or even something >> like 96061c5a4f690c3ab49e4458701bb013fd3dd57f where the close function >> assumed that several allocations were successful. These bugs existed for >> years and the fuzzer never found them. >> This means that all these errors paths need to be manually tested, so >> that something that adds error paths (like requiring things to be >> allocated) adds a burden on devs (in addition to the runtime overhead). >> >>>> >>>>>> (I actually have an idea to completely remove the packet list from >>>>>> matroskadec.) >>>>> >>>>> Should i commit this while you work on that, or just withdraw this >>>>> patch? It's not urgent at all, since any deprecation period is 2+ >>>>> years. >>>>> Either option is fine by me. >>>>> >>>> I don't want to have deprecation warnings. >>> >>> Ok. >>> >>>> >>>>>> >>>>>>> + goto fail; >>>>>>> + >>>>>>> /* The next thing is a segment. */ >>>>>>> pos = avio_tell(matroska->ctx->pb); >>>>>>> res = ebml_parse(matroska, matroska_segments, matroska); >>>>>>> @@ -2947,7 +2953,7 @@ static int >>>>>>> matroska_read_header(AVFormatContext >>>>>>> *s) >>>>>>> st->disposition |= >>>>>>> AV_DISPOSITION_ATTACHED_PIC; >>>>>>> st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO; >>>>>>> - av_init_packet(pkt); >>>>>>> + av_packet_unref(pkt); >>>>>>> pkt->buf = attachments[j].bin.buf; >>>>>>> attachments[j].bin.buf = NULL; >>>>>>> pkt->data = attachments[j].bin.data; >>>>>>> @@ -3180,7 +3186,7 @@ static int >>>>>>> matroska_parse_rm_audio(MatroskaDemuxContext *matroska, >>>>>>> while (track->audio.pkt_cnt) { >>>>>>> int ret; >>>>>>> - AVPacket pktl, *pkt = &pktl; >>>>>>> + AVPacket *pkt = matroska->pkt; >>>>>>> ret = av_new_packet(pkt, a); >>>>>>> if (ret < 0) { >>>>>>> @@ -3317,7 +3323,7 @@ static int >>>>>>> matroska_parse_webvtt(MatroskaDemuxContext *matroska, >>>>>>> uint64_t duration, >>>>>>> int64_t pos) >>>>>>> { >>>>>>> - AVPacket pktl, *pkt = &pktl; >>>>>>> + AVPacket *pkt = matroska->pkt; >>>>>>> uint8_t *id, *settings, *text, *buf; >>>>>>> int id_len, settings_len, text_len; >>>>>>> uint8_t *p, *q; >>>>>>> @@ -3434,7 +3440,7 @@ static int >>>>>>> matroska_parse_frame(MatroskaDemuxContext *matroska, >>>>>>> { >>>>>>> uint8_t *pkt_data = data; >>>>>>> int res = 0; >>>>>>> - AVPacket pktl, *pkt = &pktl; >>>>>>> + AVPacket *pkt = matroska->pkt; >>>>>>> if (st->codecpar->codec_id == AV_CODEC_ID_WAVPACK) { >>>>>>> res = matroska_parse_wavpack(track, &pkt_data, >>>>>>> &pkt_size); >>>>>>> @@ -3464,7 +3470,7 @@ static int >>>>>>> matroska_parse_frame(MatroskaDemuxContext *matroska, >>>>>>> if (!pkt_size && !additional_size) >>>>>>> goto no_output; >>>>>>> - av_init_packet(pkt); >>>>>>> + av_packet_unref(pkt); >>>>>>> if (!buf) >>>>>>> pkt->buf = av_buffer_create(pkt_data, pkt_size + >>>>>>> AV_INPUT_BUFFER_PADDING_SIZE, >>>>>>> NULL, NULL, 0); >>>>>>> @@ -3838,6 +3844,7 @@ static int >>>>>>> matroska_read_close(AVFormatContext *s) >>>>>>> int n; >>>>>>> matroska_clear_queue(matroska); >>>>>>> + av_packet_free(&matroska->pkt); >>>>>>> for (n = 0; n < matroska->tracks.nb_elem; n++) >>>>>>> if (tracks[n].type == MATROSKA_TRACK_TYPE_AUDIO) >>>>>>> >>>>>>
James Almer: > On 2/8/2021 12:22 PM, Andreas Rheinhardt wrote: >> James Almer: >>> Signed-off-by: James Almer <jamrial@gmail.com> >>> --- >>> libavformat/matroskadec.c | 17 ++++++++++++----- >>> 1 file changed, 12 insertions(+), 5 deletions(-) >>> >>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c >>> index 374831baa3..9fad78c78b 100644 >>> --- a/libavformat/matroskadec.c >>> +++ b/libavformat/matroskadec.c >>> @@ -365,6 +365,8 @@ typedef struct MatroskaDemuxContext { >>> /* byte position of the segment inside the stream */ >>> int64_t segment_start; >>> + AVPacket *pkt; >>> + >>> /* the packet queue */ >>> AVPacketList *queue; >>> AVPacketList *queue_end; >>> @@ -2885,6 +2887,10 @@ static int >>> matroska_read_header(AVFormatContext *s) >>> } >>> ebml_free(ebml_syntax, &ebml); >>> + matroska->pkt = av_packet_alloc(); >>> + if (!matroska->pkt) >> >> Missing AVERROR(ENOMEM). > > I think at this point i can just return ENOMEM without jumping to fail. Yes. - Andreas
On 2/8/2021 1:44 PM, Andreas Rheinhardt wrote: > James Almer: >> On 2/8/2021 12:57 PM, Andreas Rheinhardt wrote: >>> James Almer: >>>> On 2/8/2021 12:32 PM, Andreas Rheinhardt wrote: >>>>> James Almer: >>>>>> On 2/8/2021 12:22 PM, Andreas Rheinhardt wrote: >>>>>>> James Almer: >>>>>>>> Signed-off-by: James Almer <jamrial@gmail.com> >>>>>>>> --- >>>>>>>> libavformat/matroskadec.c | 17 ++++++++++++----- >>>>>>>> 1 file changed, 12 insertions(+), 5 deletions(-) >>>>>>>> >>>>>>>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c >>>>>>>> index 374831baa3..9fad78c78b 100644 >>>>>>>> --- a/libavformat/matroskadec.c >>>>>>>> +++ b/libavformat/matroskadec.c >>>>>>>> @@ -365,6 +365,8 @@ typedef struct MatroskaDemuxContext { >>>>>>>> /* byte position of the segment inside the stream */ >>>>>>>> int64_t segment_start; >>>>>>>> + AVPacket *pkt; >>>>>>>> + >>>>>>>> /* the packet queue */ >>>>>>>> AVPacketList *queue; >>>>>>>> AVPacketList *queue_end; >>>>>>>> @@ -2885,6 +2887,10 @@ static int >>>>>>>> matroska_read_header(AVFormatContext *s) >>>>>>>> } >>>>>>>> ebml_free(ebml_syntax, &ebml); >>>>>>>> + matroska->pkt = av_packet_alloc(); >>>>>>>> + if (!matroska->pkt) >>>>>>> >>>>>>> Missing AVERROR(ENOMEM). >>>>>> >>>>>> This seems to be a common mistake in this set. Sorry. >>>>>> >>>>> >>>>> Yeah, the failure paths seem untested. >>>> >>>> make fate doesn't test failure paths. I recall someone did some manual >>>> runs limiting available memory long ago, which detected and removed a >>>> lot of unchecked av_mallocs, but nothing automated. >>>> Fuzzing has handled failure path issue detection since then. >>>> >>> >>> No, fuzzing does not simulate low-memory environments (at least ossfuzz >>> as-is does not). >> >> I know, which is why i said failure path issues, not OOM specifically. > > Then I don't get the point of you mentioning fuzzing as handling failure > path issues in your reply to my comment about untested allocation error > paths. If you look at the above exchange, you started with the general statement "failure paths seem untested", and i continued from there. Apologies if it was confusing.
James Almer: > On 2/8/2021 1:44 PM, Andreas Rheinhardt wrote: >> James Almer: >>> On 2/8/2021 12:57 PM, Andreas Rheinhardt wrote: >>>> James Almer: >>>>> On 2/8/2021 12:32 PM, Andreas Rheinhardt wrote: >>>>>> James Almer: >>>>>>> On 2/8/2021 12:22 PM, Andreas Rheinhardt wrote: >>>>>>>> James Almer: >>>>>>>>> Signed-off-by: James Almer <jamrial@gmail.com> >>>>>>>>> --- >>>>>>>>> libavformat/matroskadec.c | 17 ++++++++++++----- >>>>>>>>> 1 file changed, 12 insertions(+), 5 deletions(-) >>>>>>>>> >>>>>>>>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c >>>>>>>>> index 374831baa3..9fad78c78b 100644 >>>>>>>>> --- a/libavformat/matroskadec.c >>>>>>>>> +++ b/libavformat/matroskadec.c >>>>>>>>> @@ -365,6 +365,8 @@ typedef struct MatroskaDemuxContext { >>>>>>>>> /* byte position of the segment inside the stream */ >>>>>>>>> int64_t segment_start; >>>>>>>>> + AVPacket *pkt; >>>>>>>>> + >>>>>>>>> /* the packet queue */ >>>>>>>>> AVPacketList *queue; >>>>>>>>> AVPacketList *queue_end; >>>>>>>>> @@ -2885,6 +2887,10 @@ static int >>>>>>>>> matroska_read_header(AVFormatContext *s) >>>>>>>>> } >>>>>>>>> ebml_free(ebml_syntax, &ebml); >>>>>>>>> + matroska->pkt = av_packet_alloc(); >>>>>>>>> + if (!matroska->pkt) >>>>>>>> >>>>>>>> Missing AVERROR(ENOMEM). >>>>>>> >>>>>>> This seems to be a common mistake in this set. Sorry. >>>>>>> >>>>>> >>>>>> Yeah, the failure paths seem untested. >>>>> >>>>> make fate doesn't test failure paths. I recall someone did some manual >>>>> runs limiting available memory long ago, which detected and removed a >>>>> lot of unchecked av_mallocs, but nothing automated. >>>>> Fuzzing has handled failure path issue detection since then. >>>>> >>>> >>>> No, fuzzing does not simulate low-memory environments (at least ossfuzz >>>> as-is does not). >>> >>> I know, which is why i said failure path issues, not OOM specifically. >> >> Then I don't get the point of you mentioning fuzzing as handling failure >> path issues in your reply to my comment about untested allocation error >> paths. > > If you look at the above exchange, you started with the general > statement "failure paths seem untested", and i continued from there. > Apologies if it was confusing. I was actually thinking about the failure paths added by your set (after all, you started the above exchange with a comment about your set); but yes, the issue is more widespread, unfortunately. Apologies for being so confusing. - Andreas
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c index 374831baa3..9fad78c78b 100644 --- a/libavformat/matroskadec.c +++ b/libavformat/matroskadec.c @@ -365,6 +365,8 @@ typedef struct MatroskaDemuxContext { /* byte position of the segment inside the stream */ int64_t segment_start; + AVPacket *pkt; + /* the packet queue */ AVPacketList *queue; AVPacketList *queue_end; @@ -2885,6 +2887,10 @@ static int matroska_read_header(AVFormatContext *s) } ebml_free(ebml_syntax, &ebml); + matroska->pkt = av_packet_alloc(); + if (!matroska->pkt) + goto fail; + /* The next thing is a segment. */ pos = avio_tell(matroska->ctx->pb); res = ebml_parse(matroska, matroska_segments, matroska); @@ -2947,7 +2953,7 @@ static int matroska_read_header(AVFormatContext *s) st->disposition |= AV_DISPOSITION_ATTACHED_PIC; st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO; - av_init_packet(pkt); + av_packet_unref(pkt); pkt->buf = attachments[j].bin.buf; attachments[j].bin.buf = NULL; pkt->data = attachments[j].bin.data; @@ -3180,7 +3186,7 @@ static int matroska_parse_rm_audio(MatroskaDemuxContext *matroska, while (track->audio.pkt_cnt) { int ret; - AVPacket pktl, *pkt = &pktl; + AVPacket *pkt = matroska->pkt; ret = av_new_packet(pkt, a); if (ret < 0) { @@ -3317,7 +3323,7 @@ static int matroska_parse_webvtt(MatroskaDemuxContext *matroska, uint64_t duration, int64_t pos) { - AVPacket pktl, *pkt = &pktl; + AVPacket *pkt = matroska->pkt; uint8_t *id, *settings, *text, *buf; int id_len, settings_len, text_len; uint8_t *p, *q; @@ -3434,7 +3440,7 @@ static int matroska_parse_frame(MatroskaDemuxContext *matroska, { uint8_t *pkt_data = data; int res = 0; - AVPacket pktl, *pkt = &pktl; + AVPacket *pkt = matroska->pkt; if (st->codecpar->codec_id == AV_CODEC_ID_WAVPACK) { res = matroska_parse_wavpack(track, &pkt_data, &pkt_size); @@ -3464,7 +3470,7 @@ static int matroska_parse_frame(MatroskaDemuxContext *matroska, if (!pkt_size && !additional_size) goto no_output; - av_init_packet(pkt); + av_packet_unref(pkt); if (!buf) pkt->buf = av_buffer_create(pkt_data, pkt_size + AV_INPUT_BUFFER_PADDING_SIZE, NULL, NULL, 0); @@ -3838,6 +3844,7 @@ static int matroska_read_close(AVFormatContext *s) int n; matroska_clear_queue(matroska); + av_packet_free(&matroska->pkt); for (n = 0; n < matroska->tracks.nb_elem; n++) if (tracks[n].type == MATROSKA_TRACK_TYPE_AUDIO)
Signed-off-by: James Almer <jamrial@gmail.com> --- libavformat/matroskadec.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)