[FFmpeg-devel] [PATCH] avformat/utils: Preserve AV_PKT_FLAG_CORRUPT

[Michael Niedermayer]

On Fri, Mar 19, 2021 at 09:02:44AM -0600, Pavel Koshevoy wrote:
> On Fri, Mar 19, 2021 at 7:58 AM Michael Niedermayer <michael at niedermayer.cc>
> wrote:
> 
> > On Wed, Mar 17, 2021 at 06:19:26PM -0600, Pavel Koshevoy wrote:
> > > Preserve AV_PKT_FLAG_CORRUPT so the caller can decide whether to drop
> > > the packet.
> > > ---
> > >  libavformat/utils.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/libavformat/utils.c b/libavformat/utils.c
> > > index a73f944e6e..0dc978e3d2 100644
> > > --- a/libavformat/utils.c
> > > +++ b/libavformat/utils.c
> > > @@ -1494,7 +1494,8 @@ static int parse_packet(AVFormatContext *s,
> > AVPacket *pkt,
> > >          out_pkt->pts          = st->parser->pts;
> > >          out_pkt->dts          = st->parser->dts;
> > >          out_pkt->pos          = st->parser->pos;
> > > -        out_pkt->flags       |= pkt->flags & AV_PKT_FLAG_DISCARD;
> > > +        out_pkt->flags       |= pkt->flags & (AV_PKT_FLAG_CORRUPT |
> > > +                                              AV_PKT_FLAG_DISCARD);
> >
> > 1. assume the parsers input are a stream of 4kb packets, the output is
> > lets say 101kb
> > packets. Assumingh one input packet is corrupted which output packet if any
> > gets the flag set and why is that the correct one ?
> >
> > 2. assume the parsers input are a stream of 128kb packets, the output is
> > lets say 3kb
> > packets. Assuming one input packet is corrupted which output packet if any
> > gets the flag set and why is that the correct one ?
> >
> 
> 
> In both cases -- any output packet that may contain any part of the
> corrupted input packet should be marked corrupted as well.

Thats a reasonable choice but i dont think thats what the patch does.
But even if this is fixed, there are still some umcomforatble cases
For example if you have a 128k input packet with 1 corrupted byte
and that is split to 128 1kb packets, marking every as corrupted
feels uncomfortable to me as 127 of them are not corrupted.
That is the meaning of AV_PKT_FLAG_CORRUPT is defined as 
"The packet content is corrupted" but in the example here in 99% of the
packets marked as corrupted there would be no corruption.

Now to make it a bit worse, we have parsers that work by reading
the next packet size and then packaging that. Corruption an happen
in that size itself. At that point the following packets can be corrupted
just because things got out of sync without any damage in the bytes of
the packets.

Maybe side data instead of a single bit flag would be better.
Specifying what range of the packet is corrupted.
For example a single corrupted 188byte input packet shouldnt cause
a 100kb size frame to be discarded, That could contain multiple
independantly decodeable slices which are known to be not corrupted


> How likely is the 2nd case?  In my case the source is a UDP mpegts stream,
> so input packets are small (188 bytes), much smaller than the typical
> output packet size.

I expect its common for audio codecs with small frames


> 
> Pavel.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20210319/f77ff42a/attachment.sig>