[FFmpeg-devel] [PATCH 2/2] avcodec/utils: keep CORRUPT and TRUSTED packet flags when parsing packets

[Michael Niedermayer]

On Wed, Oct 10, 2018 at 01:32:14AM +0200, Marton Balint wrote:
> An FF_ macro got defined in avcodec.h to store the flags which need to be
> propagated when parsers split packets so this won't be forgotten when a new
> packet flag is introduced.
> 
> (I wonder if DISPOSABLE also fits here, or maybe some special handling is
> needed like it is done for the keyframe flag?)
> ---
>  libavcodec/avcodec.h             | 9 ++++++++-
>  libavcodec/version.h             | 2 +-
>  libavformat/utils.c              | 2 +-
>  tests/ref/fate/flv-demux         | 2 +-
>  tests/ref/fate/iv8-demux         | 2 +-
>  tests/ref/fate/segment-mp4-to-ts | 6 +++---
>  tests/ref/fate/ts-demux          | 2 +-
>  7 files changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 705a3ce4f3..9a3f9b6226 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -1493,7 +1493,14 @@ typedef struct AVPacket {
>   * be discarded by the decoder.  I.e. Non-reference frames.
>   */
>  #define AV_PKT_FLAG_DISPOSABLE 0x0010
> -
> +/**
> + * Packet flags which must always be kept when parsers split packets
> + */
> +#define FF_PKT_FLAGS_KEEP_WHEN_PARSING (\
> +    AV_PKT_FLAG_CORRUPT | \
> +    AV_PKT_FLAG_DISCARD | \
> +    AV_PKT_FLAG_TRUSTED | \
> +    0)
>  
>  enum AVSideDataParamChangeFlags {
>      AV_SIDE_DATA_PARAM_CHANGE_CHANNEL_COUNT  = 0x0001,
> diff --git a/libavcodec/version.h b/libavcodec/version.h
> index 97d134851f..79c5dc6773 100644
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -29,7 +29,7 @@
>  
>  #define LIBAVCODEC_VERSION_MAJOR  58
>  #define LIBAVCODEC_VERSION_MINOR  32
> -#define LIBAVCODEC_VERSION_MICRO 100
> +#define LIBAVCODEC_VERSION_MICRO 101
>  
>  #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
>                                                 LIBAVCODEC_VERSION_MINOR, \
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index a8ac90213e..351bd88fa5 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -1511,7 +1511,7 @@ static int parse_packet(AVFormatContext *s, AVPacket *pkt, int stream_index)
>          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 & FF_PKT_FLAGS_KEEP_WHEN_PARSING;

I think this is wrong
this looks like the packet flags are not passed through the parsers, so they would
not neccessarily be attached to the correct packets, there could be a delay
from a buffer ...

more so, these flags cannot be handled identically
for example if there is output packet formed from concatenating a trusted and 
untrusted input packet the output cannot be trusted because it is not just
trusted data

compared to corrupt, if there is output packet formed from concatenating a 
corrupt and non-corrupt input packet the output still is corrupt

It gets further complicated if only part of a AV_PKT_FLAG_CORRUPT input is
used. You cant mark the output as corrupt in this case because as documented
AV_PKT_FLAG_CORRUPT means the packet IS corrupt but a subpart of it may be
corrupt or may be undamaged.
A finer granularity of corruption likelyness would be needed here.
(checksums matching, probably ok - no checksums, possibly corrupt, 
 certainly some corrupt, certain significant corruption)
Independant of this there could be information about packet truncation.
a packet content could be known to be correct but possibly incomplete
All these cases differ in how they would have to be passed on when packets
are merged / split

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

There will always be a question for which you do not know the correct answer.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20181011/76310ec0/attachment.sig>