| Message ID | 20190817194105.16492-1-cus@passwd.hu |
|---|---|
| State | Accepted |
| Commit | 6ee40dcb64c91cc9a4cb988408d8ed159dacdcfe |
| Headers | show |
lör 2019-08-17 klockan 21:41 +0200 skrev Marton Balint: > The return value was unintentionally lost after > 00a2652df3bf25a27d174cc67ed508b5317cb115. > > Signed-off-by: Marton Balint <cus@passwd.hu> > --- > libavformat/mxfdec.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c > index bb72fb9841..397f820b3f 100644 > --- a/libavformat/mxfdec.c > +++ b/libavformat/mxfdec.c > @@ -3508,8 +3508,8 @@ static int mxf_read_packet(AVFormatContext *s, AVPacket *pkt) > } else { > if ((size = next_ofs - pos) <= 0) { > av_log(s, AV_LOG_ERROR, "bad size: %"PRId64"\n", size); > - ret = AVERROR_INVALIDDATA; > - goto skip; > + mxf->current_klv_data = (KLVPacket){{0}}; > + return AVERROR_INVALIDDATA; Should maybe ask for a sample. Else looks OK /Tomas
On Sun, 18 Aug 2019, Tomas Härdin wrote: > lör 2019-08-17 klockan 21:41 +0200 skrev Marton Balint: >> The return value was unintentionally lost after >> 00a2652df3bf25a27d174cc67ed508b5317cb115. >> >> Signed-off-by: Marton Balint <cus@passwd.hu> >> --- >> libavformat/mxfdec.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c >> index bb72fb9841..397f820b3f 100644 >> --- a/libavformat/mxfdec.c >> +++ b/libavformat/mxfdec.c >> @@ -3508,8 +3508,8 @@ static int mxf_read_packet(AVFormatContext *s, AVPacket *pkt) >> } else { >> if ((size = next_ofs - pos) <= 0) { >> av_log(s, AV_LOG_ERROR, "bad size: %"PRId64"\n", size); >> - ret = AVERROR_INVALIDDATA; >> - goto skip; >> + mxf->current_klv_data = (KLVPacket){{0}}; >> + return AVERROR_INVALIDDATA; > > Should maybe ask for a sample. Else looks OK I don't think this can happen with any of the valid files, so the text saying that ffmpeg is missing a feature probably won't be true. It is a lot more likely that the user has a broken file if this error is shown. So I'd rather keep it as is. Regards, Marton
ons 2019-08-21 klockan 21:54 +0200 skrev Marton Balint: > > On Sun, 18 Aug 2019, Tomas Härdin wrote: > > > lör 2019-08-17 klockan 21:41 +0200 skrev Marton Balint: > > > The return value was unintentionally lost after > > > 00a2652df3bf25a27d174cc67ed508b5317cb115. > > > > > > Signed-off-by: Marton Balint <cus@passwd.hu> > > > --- > > > libavformat/mxfdec.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c > > > index bb72fb9841..397f820b3f 100644 > > > --- a/libavformat/mxfdec.c > > > +++ b/libavformat/mxfdec.c > > > @@ -3508,8 +3508,8 @@ static int mxf_read_packet(AVFormatContext *s, AVPacket *pkt) > > > } else { > > > if ((size = next_ofs - pos) <= 0) { > > > av_log(s, AV_LOG_ERROR, "bad size: %"PRId64"\n", size); > > > - ret = AVERROR_INVALIDDATA; > > > - goto skip; > > > + mxf->current_klv_data = (KLVPacket){{0}}; > > > + return AVERROR_INVALIDDATA; > > > > Should maybe ask for a sample. Else looks OK > > I don't think this can happen with any of the valid files, so the text > saying that ffmpeg is missing a feature probably won't be true. It is a > lot more likely that the user has a broken file if this error is shown. > > So I'd rather keep it as is. Right, the skip had me confused. This is fine of course /Tomas
On Thu, 22 Aug 2019, Tomas Härdin wrote: > ons 2019-08-21 klockan 21:54 +0200 skrev Marton Balint: >> >> On Sun, 18 Aug 2019, Tomas Härdin wrote: >> >> > lör 2019-08-17 klockan 21:41 +0200 skrev Marton Balint: >> > > The return value was unintentionally lost after >> > > 00a2652df3bf25a27d174cc67ed508b5317cb115. >> > > >> > > Signed-off-by: Marton Balint <cus@passwd.hu> >> > > --- >> > > libavformat/mxfdec.c | 4 ++-- >> > > 1 file changed, 2 insertions(+), 2 deletions(-) >> > > >> > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c >> > > index bb72fb9841..397f820b3f 100644 >> > > --- a/libavformat/mxfdec.c >> > > +++ b/libavformat/mxfdec.c >> > > @@ -3508,8 +3508,8 @@ static int mxf_read_packet(AVFormatContext *s, AVPacket *pkt) >> > > } else { >> > > if ((size = next_ofs - pos) <= 0) { >> > > av_log(s, AV_LOG_ERROR, "bad size: %"PRId64"\n", size); >> > > - ret = AVERROR_INVALIDDATA; >> > > - goto skip; >> > > + mxf->current_klv_data = (KLVPacket){{0}}; >> > > + return AVERROR_INVALIDDATA; >> > >> > Should maybe ask for a sample. Else looks OK >> >> I don't think this can happen with any of the valid files, so the text >> saying that ffmpeg is missing a feature probably won't be true. It is a >> lot more likely that the user has a broken file if this error is shown. >> >> So I'd rather keep it as is. > > Right, the skip had me confused. This is fine of course Thanks, applied. Regards, Marton
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c index bb72fb9841..397f820b3f 100644 --- a/libavformat/mxfdec.c +++ b/libavformat/mxfdec.c @@ -3508,8 +3508,8 @@ static int mxf_read_packet(AVFormatContext *s, AVPacket *pkt) } else { if ((size = next_ofs - pos) <= 0) { av_log(s, AV_LOG_ERROR, "bad size: %"PRId64"\n", size); - ret = AVERROR_INVALIDDATA; - goto skip; + mxf->current_klv_data = (KLVPacket){{0}}; + return AVERROR_INVALIDDATA; } // We must not overread, because the next edit unit might be in another KLV if (size > max_data_size)
The return value was unintentionally lost after 00a2652df3bf25a27d174cc67ed508b5317cb115. Signed-off-by: Marton Balint <cus@passwd.hu> --- libavformat/mxfdec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)