Re: [FFmpeg-devel] [PATCH 2/3] lavf/utils: add flag to discard timestamps on corrupted frames

[Michael Niedermayer]

On Wed, Dec 13, 2017 at 03:04:06AM -0600, Rodger Combs wrote:
> 
> 
> > On Dec 10, 2017, at 09:27, Michael Niedermayer  
> > wrote:
> > 
> > On Sun, Dec 10, 2017 at 05:17:44AM -0600, Rodger Combs wrote:
> >> 
> >> 
> >>> On Dec 9, 2017, at 12:19, Michael Niedermayer  
> >>> wrote:
> >>> 
> >>> On Fri, Dec 08, 2017 at 10:34:39PM -0600, Rodger Combs wrote:
>  
>  
> > On Dec 8, 2017, at 11:06, Michael Niedermayer  
> > wrote:
> > 
> > On Thu, Dec 07, 2017 at 10:23:15PM -0600, Rodger Combs wrote:
> >> ---
> >> libavformat/avformat.h  | 1 +
> >> libavformat/options_table.h | 1 +
> >> libavformat/utils.c | 8 
> >> 3 files changed, 10 insertions(+)
> >> 
> >> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> >> index 4f2798a871..d10d583dff 100644
> >> --- a/libavformat/avformat.h
> >> +++ b/libavformat/avformat.h
> >> @@ -1450,6 +1450,7 @@ typedef struct AVFormatContext {
> >> #define AVFMT_FLAG_FAST_SEEK   0x8 ///< Enable fast, but 
> >> inaccurate seeks for some formats
> >> #define AVFMT_FLAG_SHORTEST   0x10 ///< Stop muxing when the 
> >> shortest stream stops.
> >> #define AVFMT_FLAG_AUTO_BSF   0x20 ///< Add bitstream filters as 
> >> requested by the muxer
> >> +#define AVFMT_FLAG_DISCARD_CORRUPT_TS 0x40 ///< Discard 
> >> timestamps of frames marked corrupt
> >> 
> >>   /**
> >>* Maximum size of the data read from input for determining
> >> diff --git a/libavformat/options_table.h b/libavformat/options_table.h
> >> index b8fa47c6fd..515574d3e0 100644
> >> --- a/libavformat/options_table.h
> >> +++ b/libavformat/options_table.h
> >> @@ -58,6 +58,7 @@ static const AVOption avformat_options[] = {
> >> {"bitexact", "do not write random/volatile data", 0, 
> >> AV_OPT_TYPE_CONST, { .i64 = AVFMT_FLAG_BITEXACT }, 0, 0, E, "fflags" },
> >> {"shortest", "stop muxing with the shortest stream", 0, 
> >> AV_OPT_TYPE_CONST, { .i64 = AVFMT_FLAG_SHORTEST }, 0, 0, E, "fflags" },
> >> {"autobsf", "add needed bsfs automatically", 0, AV_OPT_TYPE_CONST, { 
> >> .i64 = AVFMT_FLAG_AUTO_BSF }, 0, 0, E, "fflags" },
> >> +{"discardcorruptts", "discard timestamps on corrupted frames", 0, 
> >> AV_OPT_TYPE_CONST, { .i64 = AVFMT_FLAG_DISCARD_CORRUPT_TS }, 0, 0, E, 
> >> "fflags" },
> >> {"analyzeduration", "specify how many microseconds are analyzed to 
> >> probe the input", OFFSET(max_analyze_duration), AV_OPT_TYPE_INT64, 
> >> {.i64 = 0 }, 0, INT64_MAX, D},
> >> {"cryptokey", "decryption key", OFFSET(key), AV_OPT_TYPE_BINARY, {.dbl 
> >> = 0}, 0, 0, D},
> >> {"indexmem", "max memory used for timestamp index (per stream)", 
> >> OFFSET(max_index_size), AV_OPT_TYPE_INT, {.i64 = 1<<20 }, 0, INT_MAX, 
> >> D},
> >> diff --git a/libavformat/utils.c b/libavformat/utils.c
> >> index 84e49208b8..98af941e9f 100644
> >> --- a/libavformat/utils.c
> >> +++ b/libavformat/utils.c
> > 
> >> @@ -873,6 +873,14 @@ int ff_read_packet(AVFormatContext *s, AVPacket 
> >> *pkt)
> >>   st->cur_dts = wrap_timestamp(st, st->cur_dts);
> >>   }
> >> 
> >> +if ((s->flags & AVFMT_FLAG_DISCARD_CORRUPT_TS) &&
> >> +(pkt->flags & AV_PKT_FLAG_CORRUPT)) {
> >> +pkt->pts = pkt->dts = AV_NOPTS_VALUE;
> >> +av_log(s, AV_LOG_WARNING,
> >> +   "Discarded timestamp on corrupted packet (stream = 
> >> %d)\n",
> >> +   pkt->stream_index);
> >> +}
> > 
> > how many of the cases that set AV_PKT_FLAG_CORRUPT can even be due to
> > the timestamp ?
>  
>  Pretty much just the new TEI check, or potentially other CRC checks. I 
>  don't _think_ the continuity-counter check failing could indicate an 
>  incorrect timestamp. I suppose we could add a second flag just for 
>  corruption that could affect timestamps?
> >>> 
> >>> I think thats a great idea, maybe if we already change it we should
> >>> also split the truncated case out as it is very common.
> >>> 
> >>> AV_PKT_FLAG_DATA_CORRUPT// the packet data may be 
> >>> corrupted
> >>> AV_PKT_FLAG_DATA_TRUNCATED  // the packet size may be 
> >>> corrupted, data available is undamaged unless another flag indicates 
> >>> otherwise. Any headers or sub-packets which are complete are non 
> >>> corrupted-
> >>> AV_PKT_FLAG_TS_CORRUPT  // the packets timestamps and or 
> >>> duration may be corrupted
> >>> 
> >>> #define AV_PKT_FLAG_CORRUPT (AV_PKT_FLAG_DATA_CORRUPT | 
> >>> AV_PKT_FLAG_DATA_TRUNCATED | AV_PKT_FLAG_TS_CORRUPT)
> >> 
> >> What kind of API bump would be needed to make this kind of change?
> > 
> > IIUC we are still within the major bump thing (didnt check 

Re: [FFmpeg-devel] [PATCH 2/3] lavf/utils: add flag to discard timestamps on corrupted frames

[Rodger Combs]

> On Dec 10, 2017, at 09:27, Michael Niedermayer  wrote:
> 
> On Sun, Dec 10, 2017 at 05:17:44AM -0600, Rodger Combs wrote:
>> 
>> 
>>> On Dec 9, 2017, at 12:19, Michael Niedermayer  
>>> wrote:
>>> 
>>> On Fri, Dec 08, 2017 at 10:34:39PM -0600, Rodger Combs wrote:
 
 
> On Dec 8, 2017, at 11:06, Michael Niedermayer  
> wrote:
> 
> On Thu, Dec 07, 2017 at 10:23:15PM -0600, Rodger Combs wrote:
>> ---
>> libavformat/avformat.h  | 1 +
>> libavformat/options_table.h | 1 +
>> libavformat/utils.c | 8 
>> 3 files changed, 10 insertions(+)
>> 
>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>> index 4f2798a871..d10d583dff 100644
>> --- a/libavformat/avformat.h
>> +++ b/libavformat/avformat.h
>> @@ -1450,6 +1450,7 @@ typedef struct AVFormatContext {
>> #define AVFMT_FLAG_FAST_SEEK   0x8 ///< Enable fast, but inaccurate 
>> seeks for some formats
>> #define AVFMT_FLAG_SHORTEST   0x10 ///< Stop muxing when the 
>> shortest stream stops.
>> #define AVFMT_FLAG_AUTO_BSF   0x20 ///< Add bitstream filters as 
>> requested by the muxer
>> +#define AVFMT_FLAG_DISCARD_CORRUPT_TS 0x40 ///< Discard timestamps 
>> of frames marked corrupt
>> 
>>   /**
>>* Maximum size of the data read from input for determining
>> diff --git a/libavformat/options_table.h b/libavformat/options_table.h
>> index b8fa47c6fd..515574d3e0 100644
>> --- a/libavformat/options_table.h
>> +++ b/libavformat/options_table.h
>> @@ -58,6 +58,7 @@ static const AVOption avformat_options[] = {
>> {"bitexact", "do not write random/volatile data", 0, AV_OPT_TYPE_CONST, 
>> { .i64 = AVFMT_FLAG_BITEXACT }, 0, 0, E, "fflags" },
>> {"shortest", "stop muxing with the shortest stream", 0, 
>> AV_OPT_TYPE_CONST, { .i64 = AVFMT_FLAG_SHORTEST }, 0, 0, E, "fflags" },
>> {"autobsf", "add needed bsfs automatically", 0, AV_OPT_TYPE_CONST, { 
>> .i64 = AVFMT_FLAG_AUTO_BSF }, 0, 0, E, "fflags" },
>> +{"discardcorruptts", "discard timestamps on corrupted frames", 0, 
>> AV_OPT_TYPE_CONST, { .i64 = AVFMT_FLAG_DISCARD_CORRUPT_TS }, 0, 0, E, 
>> "fflags" },
>> {"analyzeduration", "specify how many microseconds are analyzed to probe 
>> the input", OFFSET(max_analyze_duration), AV_OPT_TYPE_INT64, {.i64 = 0 
>> }, 0, INT64_MAX, D},
>> {"cryptokey", "decryption key", OFFSET(key), AV_OPT_TYPE_BINARY, {.dbl = 
>> 0}, 0, 0, D},
>> {"indexmem", "max memory used for timestamp index (per stream)", 
>> OFFSET(max_index_size), AV_OPT_TYPE_INT, {.i64 = 1<<20 }, 0, INT_MAX, D},
>> diff --git a/libavformat/utils.c b/libavformat/utils.c
>> index 84e49208b8..98af941e9f 100644
>> --- a/libavformat/utils.c
>> +++ b/libavformat/utils.c
> 
>> @@ -873,6 +873,14 @@ int ff_read_packet(AVFormatContext *s, AVPacket 
>> *pkt)
>>   st->cur_dts = wrap_timestamp(st, st->cur_dts);
>>   }
>> 
>> +if ((s->flags & AVFMT_FLAG_DISCARD_CORRUPT_TS) &&
>> +(pkt->flags & AV_PKT_FLAG_CORRUPT)) {
>> +pkt->pts = pkt->dts = AV_NOPTS_VALUE;
>> +av_log(s, AV_LOG_WARNING,
>> +   "Discarded timestamp on corrupted packet (stream = 
>> %d)\n",
>> +   pkt->stream_index);
>> +}
> 
> how many of the cases that set AV_PKT_FLAG_CORRUPT can even be due to
> the timestamp ?
 
 Pretty much just the new TEI check, or potentially other CRC checks. I 
 don't _think_ the continuity-counter check failing could indicate an 
 incorrect timestamp. I suppose we could add a second flag just for 
 corruption that could affect timestamps?
>>> 
>>> I think thats a great idea, maybe if we already change it we should
>>> also split the truncated case out as it is very common.
>>> 
>>> AV_PKT_FLAG_DATA_CORRUPT// the packet data may be corrupted
>>> AV_PKT_FLAG_DATA_TRUNCATED  // the packet size may be 
>>> corrupted, data available is undamaged unless another flag indicates 
>>> otherwise. Any headers or sub-packets which are complete are non corrupted-
>>> AV_PKT_FLAG_TS_CORRUPT  // the packets timestamps and or 
>>> duration may be corrupted
>>> 
>>> #define AV_PKT_FLAG_CORRUPT (AV_PKT_FLAG_DATA_CORRUPT | 
>>> AV_PKT_FLAG_DATA_TRUNCATED | AV_PKT_FLAG_TS_CORRUPT)
>> 
>> What kind of API bump would be needed to make this kind of change?
> 
> IIUC we are still within the major bump thing (didnt check but others
> have said it in relation to other changes)
> 
> a minor bump should probably be done anyway to signal that these
> additional flag may be set in the output

Come to think of it, I think a major bump would be required, as this would 
break ABI (applications checking for 

Re: [FFmpeg-devel] [PATCH 2/3] lavf/utils: add flag to discard timestamps on corrupted frames

[Michael Niedermayer]

On Sun, Dec 10, 2017 at 04:30:25PM +0100, Nicolas George wrote:
> Michael Niedermayer (2017-12-10):
> > IIUC we are still within the major bump thing (didnt check but others
> > have said it in relation to other changes)
> 
> Well, there are no laws that say when we leave the "major bump thing".
> We dice how long it lasts.



> 
> I can suggest this: versions with minor 0 have an unstable ABI; we mark
> the end of the "major bump thing" by bumping the minor.

we could do this next time, we did not do this in the past and cannot
now as minor has already been bumped
also this would add the restriction that we could not do any versioning
during this period which would be a bad idea IMHO


[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In fact, the RIAA has been known to suggest that students drop out
of college or go to community college in order to be able to afford
settlements. -- The RIAA


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/3] lavf/utils: add flag to discard timestamps on corrupted frames

[Nicolas George]

Michael Niedermayer (2017-12-10):
> IIUC we are still within the major bump thing (didnt check but others
> have said it in relation to other changes)

Well, there are no laws that say when we leave the "major bump thing".
We dice how long it lasts.

I can suggest this: versions with minor 0 have an unstable ABI; we mark
the end of the "major bump thing" by bumping the minor.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/3] lavf/utils: add flag to discard timestamps on corrupted frames

[Michael Niedermayer]

On Sun, Dec 10, 2017 at 05:17:44AM -0600, Rodger Combs wrote:
> 
> 
> > On Dec 9, 2017, at 12:19, Michael Niedermayer  
> > wrote:
> > 
> > On Fri, Dec 08, 2017 at 10:34:39PM -0600, Rodger Combs wrote:
> >> 
> >> 
> >>> On Dec 8, 2017, at 11:06, Michael Niedermayer  
> >>> wrote:
> >>> 
> >>> On Thu, Dec 07, 2017 at 10:23:15PM -0600, Rodger Combs wrote:
>  ---
>  libavformat/avformat.h  | 1 +
>  libavformat/options_table.h | 1 +
>  libavformat/utils.c | 8 
>  3 files changed, 10 insertions(+)
>  
>  diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>  index 4f2798a871..d10d583dff 100644
>  --- a/libavformat/avformat.h
>  +++ b/libavformat/avformat.h
>  @@ -1450,6 +1450,7 @@ typedef struct AVFormatContext {
>  #define AVFMT_FLAG_FAST_SEEK   0x8 ///< Enable fast, but inaccurate 
>  seeks for some formats
>  #define AVFMT_FLAG_SHORTEST   0x10 ///< Stop muxing when the 
>  shortest stream stops.
>  #define AVFMT_FLAG_AUTO_BSF   0x20 ///< Add bitstream filters as 
>  requested by the muxer
>  +#define AVFMT_FLAG_DISCARD_CORRUPT_TS 0x40 ///< Discard timestamps 
>  of frames marked corrupt
>  
> /**
>  * Maximum size of the data read from input for determining
>  diff --git a/libavformat/options_table.h b/libavformat/options_table.h
>  index b8fa47c6fd..515574d3e0 100644
>  --- a/libavformat/options_table.h
>  +++ b/libavformat/options_table.h
>  @@ -58,6 +58,7 @@ static const AVOption avformat_options[] = {
>  {"bitexact", "do not write random/volatile data", 0, AV_OPT_TYPE_CONST, 
>  { .i64 = AVFMT_FLAG_BITEXACT }, 0, 0, E, "fflags" },
>  {"shortest", "stop muxing with the shortest stream", 0, 
>  AV_OPT_TYPE_CONST, { .i64 = AVFMT_FLAG_SHORTEST }, 0, 0, E, "fflags" },
>  {"autobsf", "add needed bsfs automatically", 0, AV_OPT_TYPE_CONST, { 
>  .i64 = AVFMT_FLAG_AUTO_BSF }, 0, 0, E, "fflags" },
>  +{"discardcorruptts", "discard timestamps on corrupted frames", 0, 
>  AV_OPT_TYPE_CONST, { .i64 = AVFMT_FLAG_DISCARD_CORRUPT_TS }, 0, 0, E, 
>  "fflags" },
>  {"analyzeduration", "specify how many microseconds are analyzed to probe 
>  the input", OFFSET(max_analyze_duration), AV_OPT_TYPE_INT64, {.i64 = 0 
>  }, 0, INT64_MAX, D},
>  {"cryptokey", "decryption key", OFFSET(key), AV_OPT_TYPE_BINARY, {.dbl = 
>  0}, 0, 0, D},
>  {"indexmem", "max memory used for timestamp index (per stream)", 
>  OFFSET(max_index_size), AV_OPT_TYPE_INT, {.i64 = 1<<20 }, 0, INT_MAX, D},
>  diff --git a/libavformat/utils.c b/libavformat/utils.c
>  index 84e49208b8..98af941e9f 100644
>  --- a/libavformat/utils.c
>  +++ b/libavformat/utils.c
> >>> 
>  @@ -873,6 +873,14 @@ int ff_read_packet(AVFormatContext *s, AVPacket 
>  *pkt)
> st->cur_dts = wrap_timestamp(st, st->cur_dts);
> }
>  
>  +if ((s->flags & AVFMT_FLAG_DISCARD_CORRUPT_TS) &&
>  +(pkt->flags & AV_PKT_FLAG_CORRUPT)) {
>  +pkt->pts = pkt->dts = AV_NOPTS_VALUE;
>  +av_log(s, AV_LOG_WARNING,
>  +   "Discarded timestamp on corrupted packet (stream = 
>  %d)\n",
>  +   pkt->stream_index);
>  +}
> >>> 
> >>> how many of the cases that set AV_PKT_FLAG_CORRUPT can even be due to
> >>> the timestamp ?
> >> 
> >> Pretty much just the new TEI check, or potentially other CRC checks. I 
> >> don't _think_ the continuity-counter check failing could indicate an 
> >> incorrect timestamp. I suppose we could add a second flag just for 
> >> corruption that could affect timestamps?
> > 
> > I think thats a great idea, maybe if we already change it we should
> > also split the truncated case out as it is very common.
> > 
> > AV_PKT_FLAG_DATA_CORRUPT// the packet data may be corrupted
> > AV_PKT_FLAG_DATA_TRUNCATED  // the packet size may be 
> > corrupted, data available is undamaged unless another flag indicates 
> > otherwise. Any headers or sub-packets which are complete are non corrupted-
> > AV_PKT_FLAG_TS_CORRUPT  // the packets timestamps and or 
> > duration may be corrupted
> > 
> > #define AV_PKT_FLAG_CORRUPT (AV_PKT_FLAG_DATA_CORRUPT | 
> > AV_PKT_FLAG_DATA_TRUNCATED | AV_PKT_FLAG_TS_CORRUPT)
> 
> What kind of API bump would be needed to make this kind of change?

IIUC we are still within the major bump thing (didnt check but others
have said it in relation to other changes)

a minor bump should probably be done anyway to signal that these
additional flag may be set in the output


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

Complexity theory is the science of finding the exact solution to an
approximation. Benchmarking OTOH is finding an approximation of the exact

Re: [FFmpeg-devel] [PATCH 2/3] lavf/utils: add flag to discard timestamps on corrupted frames

[Rodger Combs]

> On Dec 9, 2017, at 12:19, Michael Niedermayer  wrote:
> 
> On Fri, Dec 08, 2017 at 10:34:39PM -0600, Rodger Combs wrote:
>> 
>> 
>>> On Dec 8, 2017, at 11:06, Michael Niedermayer  
>>> wrote:
>>> 
>>> On Thu, Dec 07, 2017 at 10:23:15PM -0600, Rodger Combs wrote:
 ---
 libavformat/avformat.h  | 1 +
 libavformat/options_table.h | 1 +
 libavformat/utils.c | 8 
 3 files changed, 10 insertions(+)
 
 diff --git a/libavformat/avformat.h b/libavformat/avformat.h
 index 4f2798a871..d10d583dff 100644
 --- a/libavformat/avformat.h
 +++ b/libavformat/avformat.h
 @@ -1450,6 +1450,7 @@ typedef struct AVFormatContext {
 #define AVFMT_FLAG_FAST_SEEK   0x8 ///< Enable fast, but inaccurate 
 seeks for some formats
 #define AVFMT_FLAG_SHORTEST   0x10 ///< Stop muxing when the shortest 
 stream stops.
 #define AVFMT_FLAG_AUTO_BSF   0x20 ///< Add bitstream filters as 
 requested by the muxer
 +#define AVFMT_FLAG_DISCARD_CORRUPT_TS 0x40 ///< Discard timestamps of 
 frames marked corrupt
 
/**
 * Maximum size of the data read from input for determining
 diff --git a/libavformat/options_table.h b/libavformat/options_table.h
 index b8fa47c6fd..515574d3e0 100644
 --- a/libavformat/options_table.h
 +++ b/libavformat/options_table.h
 @@ -58,6 +58,7 @@ static const AVOption avformat_options[] = {
 {"bitexact", "do not write random/volatile data", 0, AV_OPT_TYPE_CONST, { 
 .i64 = AVFMT_FLAG_BITEXACT }, 0, 0, E, "fflags" },
 {"shortest", "stop muxing with the shortest stream", 0, AV_OPT_TYPE_CONST, 
 { .i64 = AVFMT_FLAG_SHORTEST }, 0, 0, E, "fflags" },
 {"autobsf", "add needed bsfs automatically", 0, AV_OPT_TYPE_CONST, { .i64 
 = AVFMT_FLAG_AUTO_BSF }, 0, 0, E, "fflags" },
 +{"discardcorruptts", "discard timestamps on corrupted frames", 0, 
 AV_OPT_TYPE_CONST, { .i64 = AVFMT_FLAG_DISCARD_CORRUPT_TS }, 0, 0, E, 
 "fflags" },
 {"analyzeduration", "specify how many microseconds are analyzed to probe 
 the input", OFFSET(max_analyze_duration), AV_OPT_TYPE_INT64, {.i64 = 0 }, 
 0, INT64_MAX, D},
 {"cryptokey", "decryption key", OFFSET(key), AV_OPT_TYPE_BINARY, {.dbl = 
 0}, 0, 0, D},
 {"indexmem", "max memory used for timestamp index (per stream)", 
 OFFSET(max_index_size), AV_OPT_TYPE_INT, {.i64 = 1<<20 }, 0, INT_MAX, D},
 diff --git a/libavformat/utils.c b/libavformat/utils.c
 index 84e49208b8..98af941e9f 100644
 --- a/libavformat/utils.c
 +++ b/libavformat/utils.c
>>> 
 @@ -873,6 +873,14 @@ int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
st->cur_dts = wrap_timestamp(st, st->cur_dts);
}
 
 +if ((s->flags & AVFMT_FLAG_DISCARD_CORRUPT_TS) &&
 +(pkt->flags & AV_PKT_FLAG_CORRUPT)) {
 +pkt->pts = pkt->dts = AV_NOPTS_VALUE;
 +av_log(s, AV_LOG_WARNING,
 +   "Discarded timestamp on corrupted packet (stream = 
 %d)\n",
 +   pkt->stream_index);
 +}
>>> 
>>> how many of the cases that set AV_PKT_FLAG_CORRUPT can even be due to
>>> the timestamp ?
>> 
>> Pretty much just the new TEI check, or potentially other CRC checks. I don't 
>> _think_ the continuity-counter check failing could indicate an incorrect 
>> timestamp. I suppose we could add a second flag just for corruption that 
>> could affect timestamps?
> 
> I think thats a great idea, maybe if we already change it we should
> also split the truncated case out as it is very common.
> 
> AV_PKT_FLAG_DATA_CORRUPT// the packet data may be corrupted
> AV_PKT_FLAG_DATA_TRUNCATED  // the packet size may be corrupted, 
> data available is undamaged unless another flag indicates otherwise. Any 
> headers or sub-packets which are complete are non corrupted-
> AV_PKT_FLAG_TS_CORRUPT  // the packets timestamps and or 
> duration may be corrupted
> 
> #define AV_PKT_FLAG_CORRUPT (AV_PKT_FLAG_DATA_CORRUPT | 
> AV_PKT_FLAG_DATA_TRUNCATED | AV_PKT_FLAG_TS_CORRUPT)

What kind of API bump would be needed to make this kind of change?

> 
> or some variation of this
> 
> Thanks
> 
> [...]
> -- 
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> There will always be a question for which you do not know the correct answer.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org 
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel 
> 
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/3] lavf/utils: add flag to discard timestamps on corrupted frames

[Michael Niedermayer]

On Fri, Dec 08, 2017 at 10:34:39PM -0600, Rodger Combs wrote:
> 
> 
> > On Dec 8, 2017, at 11:06, Michael Niedermayer  
> > wrote:
> > 
> > On Thu, Dec 07, 2017 at 10:23:15PM -0600, Rodger Combs wrote:
> >> ---
> >> libavformat/avformat.h  | 1 +
> >> libavformat/options_table.h | 1 +
> >> libavformat/utils.c | 8 
> >> 3 files changed, 10 insertions(+)
> >> 
> >> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> >> index 4f2798a871..d10d583dff 100644
> >> --- a/libavformat/avformat.h
> >> +++ b/libavformat/avformat.h
> >> @@ -1450,6 +1450,7 @@ typedef struct AVFormatContext {
> >> #define AVFMT_FLAG_FAST_SEEK   0x8 ///< Enable fast, but inaccurate 
> >> seeks for some formats
> >> #define AVFMT_FLAG_SHORTEST   0x10 ///< Stop muxing when the shortest 
> >> stream stops.
> >> #define AVFMT_FLAG_AUTO_BSF   0x20 ///< Add bitstream filters as 
> >> requested by the muxer
> >> +#define AVFMT_FLAG_DISCARD_CORRUPT_TS 0x40 ///< Discard timestamps of 
> >> frames marked corrupt
> >> 
> >> /**
> >>  * Maximum size of the data read from input for determining
> >> diff --git a/libavformat/options_table.h b/libavformat/options_table.h
> >> index b8fa47c6fd..515574d3e0 100644
> >> --- a/libavformat/options_table.h
> >> +++ b/libavformat/options_table.h
> >> @@ -58,6 +58,7 @@ static const AVOption avformat_options[] = {
> >> {"bitexact", "do not write random/volatile data", 0, AV_OPT_TYPE_CONST, { 
> >> .i64 = AVFMT_FLAG_BITEXACT }, 0, 0, E, "fflags" },
> >> {"shortest", "stop muxing with the shortest stream", 0, AV_OPT_TYPE_CONST, 
> >> { .i64 = AVFMT_FLAG_SHORTEST }, 0, 0, E, "fflags" },
> >> {"autobsf", "add needed bsfs automatically", 0, AV_OPT_TYPE_CONST, { .i64 
> >> = AVFMT_FLAG_AUTO_BSF }, 0, 0, E, "fflags" },
> >> +{"discardcorruptts", "discard timestamps on corrupted frames", 0, 
> >> AV_OPT_TYPE_CONST, { .i64 = AVFMT_FLAG_DISCARD_CORRUPT_TS }, 0, 0, E, 
> >> "fflags" },
> >> {"analyzeduration", "specify how many microseconds are analyzed to probe 
> >> the input", OFFSET(max_analyze_duration), AV_OPT_TYPE_INT64, {.i64 = 0 }, 
> >> 0, INT64_MAX, D},
> >> {"cryptokey", "decryption key", OFFSET(key), AV_OPT_TYPE_BINARY, {.dbl = 
> >> 0}, 0, 0, D},
> >> {"indexmem", "max memory used for timestamp index (per stream)", 
> >> OFFSET(max_index_size), AV_OPT_TYPE_INT, {.i64 = 1<<20 }, 0, INT_MAX, D},
> >> diff --git a/libavformat/utils.c b/libavformat/utils.c
> >> index 84e49208b8..98af941e9f 100644
> >> --- a/libavformat/utils.c
> >> +++ b/libavformat/utils.c
> > 
> >> @@ -873,6 +873,14 @@ int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
> >> st->cur_dts = wrap_timestamp(st, st->cur_dts);
> >> }
> >> 
> >> +if ((s->flags & AVFMT_FLAG_DISCARD_CORRUPT_TS) &&
> >> +(pkt->flags & AV_PKT_FLAG_CORRUPT)) {
> >> +pkt->pts = pkt->dts = AV_NOPTS_VALUE;
> >> +av_log(s, AV_LOG_WARNING,
> >> +   "Discarded timestamp on corrupted packet (stream = 
> >> %d)\n",
> >> +   pkt->stream_index);
> >> +}
> > 
> > how many of the cases that set AV_PKT_FLAG_CORRUPT can even be due to
> > the timestamp ?
> 
> Pretty much just the new TEI check, or potentially other CRC checks. I don't 
> _think_ the continuity-counter check failing could indicate an incorrect 
> timestamp. I suppose we could add a second flag just for corruption that 
> could affect timestamps?

I think thats a great idea, maybe if we already change it we should
also split the truncated case out as it is very common.

AV_PKT_FLAG_DATA_CORRUPT// the packet data may be corrupted
AV_PKT_FLAG_DATA_TRUNCATED  // the packet size may be corrupted, 
data available is undamaged unless another flag indicates otherwise. Any 
headers or sub-packets which are complete are non corrupted-
AV_PKT_FLAG_TS_CORRUPT  // the packets timestamps and or 
duration may be corrupted

#define AV_PKT_FLAG_CORRUPT (AV_PKT_FLAG_DATA_CORRUPT | 
AV_PKT_FLAG_DATA_TRUNCATED | AV_PKT_FLAG_TS_CORRUPT)

or some variation of this

Thanks

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

There will always be a question for which you do not know the correct answer.


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/3] lavf/utils: add flag to discard timestamps on corrupted frames

[Rodger Combs]

> On Dec 8, 2017, at 11:06, Michael Niedermayer  wrote:
> 
> On Thu, Dec 07, 2017 at 10:23:15PM -0600, Rodger Combs wrote:
>> ---
>> libavformat/avformat.h  | 1 +
>> libavformat/options_table.h | 1 +
>> libavformat/utils.c | 8 
>> 3 files changed, 10 insertions(+)
>> 
>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>> index 4f2798a871..d10d583dff 100644
>> --- a/libavformat/avformat.h
>> +++ b/libavformat/avformat.h
>> @@ -1450,6 +1450,7 @@ typedef struct AVFormatContext {
>> #define AVFMT_FLAG_FAST_SEEK   0x8 ///< Enable fast, but inaccurate 
>> seeks for some formats
>> #define AVFMT_FLAG_SHORTEST   0x10 ///< Stop muxing when the shortest 
>> stream stops.
>> #define AVFMT_FLAG_AUTO_BSF   0x20 ///< Add bitstream filters as 
>> requested by the muxer
>> +#define AVFMT_FLAG_DISCARD_CORRUPT_TS 0x40 ///< Discard timestamps of 
>> frames marked corrupt
>> 
>> /**
>>  * Maximum size of the data read from input for determining
>> diff --git a/libavformat/options_table.h b/libavformat/options_table.h
>> index b8fa47c6fd..515574d3e0 100644
>> --- a/libavformat/options_table.h
>> +++ b/libavformat/options_table.h
>> @@ -58,6 +58,7 @@ static const AVOption avformat_options[] = {
>> {"bitexact", "do not write random/volatile data", 0, AV_OPT_TYPE_CONST, { 
>> .i64 = AVFMT_FLAG_BITEXACT }, 0, 0, E, "fflags" },
>> {"shortest", "stop muxing with the shortest stream", 0, AV_OPT_TYPE_CONST, { 
>> .i64 = AVFMT_FLAG_SHORTEST }, 0, 0, E, "fflags" },
>> {"autobsf", "add needed bsfs automatically", 0, AV_OPT_TYPE_CONST, { .i64 = 
>> AVFMT_FLAG_AUTO_BSF }, 0, 0, E, "fflags" },
>> +{"discardcorruptts", "discard timestamps on corrupted frames", 0, 
>> AV_OPT_TYPE_CONST, { .i64 = AVFMT_FLAG_DISCARD_CORRUPT_TS }, 0, 0, E, 
>> "fflags" },
>> {"analyzeduration", "specify how many microseconds are analyzed to probe the 
>> input", OFFSET(max_analyze_duration), AV_OPT_TYPE_INT64, {.i64 = 0 }, 0, 
>> INT64_MAX, D},
>> {"cryptokey", "decryption key", OFFSET(key), AV_OPT_TYPE_BINARY, {.dbl = 0}, 
>> 0, 0, D},
>> {"indexmem", "max memory used for timestamp index (per stream)", 
>> OFFSET(max_index_size), AV_OPT_TYPE_INT, {.i64 = 1<<20 }, 0, INT_MAX, D},
>> diff --git a/libavformat/utils.c b/libavformat/utils.c
>> index 84e49208b8..98af941e9f 100644
>> --- a/libavformat/utils.c
>> +++ b/libavformat/utils.c
> 
>> @@ -873,6 +873,14 @@ int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
>> st->cur_dts = wrap_timestamp(st, st->cur_dts);
>> }
>> 
>> +if ((s->flags & AVFMT_FLAG_DISCARD_CORRUPT_TS) &&
>> +(pkt->flags & AV_PKT_FLAG_CORRUPT)) {
>> +pkt->pts = pkt->dts = AV_NOPTS_VALUE;
>> +av_log(s, AV_LOG_WARNING,
>> +   "Discarded timestamp on corrupted packet (stream = 
>> %d)\n",
>> +   pkt->stream_index);
>> +}
> 
> how many of the cases that set AV_PKT_FLAG_CORRUPT can even be due to
> the timestamp ?

Pretty much just the new TEI check, or potentially other CRC checks. I don't 
_think_ the continuity-counter check failing could indicate an incorrect 
timestamp. I suppose we could add a second flag just for corruption that could 
affect timestamps?

> 
> We set this for truncated / EOF cases and all kinds of stuff that
> are known to be unrelated to the timestamps
> 
> 
> [...]
> 
> -- 
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> The real ebay dictionary, page 1
> "Used only once"- "Some unspecified defect prevented a second use"
> "In good condition" - "Can be repaird by experienced expert"
> "As is" - "You wouldnt want it even if you were payed for it, if you knew ..."
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org 
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel 
> 
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/3] lavf/utils: add flag to discard timestamps on corrupted frames

[Michael Niedermayer]

On Thu, Dec 07, 2017 at 10:23:15PM -0600, Rodger Combs wrote:
> ---
>  libavformat/avformat.h  | 1 +
>  libavformat/options_table.h | 1 +
>  libavformat/utils.c | 8 
>  3 files changed, 10 insertions(+)
> 
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 4f2798a871..d10d583dff 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -1450,6 +1450,7 @@ typedef struct AVFormatContext {
>  #define AVFMT_FLAG_FAST_SEEK   0x8 ///< Enable fast, but inaccurate 
> seeks for some formats
>  #define AVFMT_FLAG_SHORTEST   0x10 ///< Stop muxing when the shortest 
> stream stops.
>  #define AVFMT_FLAG_AUTO_BSF   0x20 ///< Add bitstream filters as 
> requested by the muxer
> +#define AVFMT_FLAG_DISCARD_CORRUPT_TS 0x40 ///< Discard timestamps of 
> frames marked corrupt
>  
>  /**
>   * Maximum size of the data read from input for determining
> diff --git a/libavformat/options_table.h b/libavformat/options_table.h
> index b8fa47c6fd..515574d3e0 100644
> --- a/libavformat/options_table.h
> +++ b/libavformat/options_table.h
> @@ -58,6 +58,7 @@ static const AVOption avformat_options[] = {
>  {"bitexact", "do not write random/volatile data", 0, AV_OPT_TYPE_CONST, { 
> .i64 = AVFMT_FLAG_BITEXACT }, 0, 0, E, "fflags" },
>  {"shortest", "stop muxing with the shortest stream", 0, AV_OPT_TYPE_CONST, { 
> .i64 = AVFMT_FLAG_SHORTEST }, 0, 0, E, "fflags" },
>  {"autobsf", "add needed bsfs automatically", 0, AV_OPT_TYPE_CONST, { .i64 = 
> AVFMT_FLAG_AUTO_BSF }, 0, 0, E, "fflags" },
> +{"discardcorruptts", "discard timestamps on corrupted frames", 0, 
> AV_OPT_TYPE_CONST, { .i64 = AVFMT_FLAG_DISCARD_CORRUPT_TS }, 0, 0, E, 
> "fflags" },
>  {"analyzeduration", "specify how many microseconds are analyzed to probe the 
> input", OFFSET(max_analyze_duration), AV_OPT_TYPE_INT64, {.i64 = 0 }, 0, 
> INT64_MAX, D},
>  {"cryptokey", "decryption key", OFFSET(key), AV_OPT_TYPE_BINARY, {.dbl = 0}, 
> 0, 0, D},
>  {"indexmem", "max memory used for timestamp index (per stream)", 
> OFFSET(max_index_size), AV_OPT_TYPE_INT, {.i64 = 1<<20 }, 0, INT_MAX, D},
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 84e49208b8..98af941e9f 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c

> @@ -873,6 +873,14 @@ int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
>  st->cur_dts = wrap_timestamp(st, st->cur_dts);
>  }
>  
> +if ((s->flags & AVFMT_FLAG_DISCARD_CORRUPT_TS) &&
> +(pkt->flags & AV_PKT_FLAG_CORRUPT)) {
> +pkt->pts = pkt->dts = AV_NOPTS_VALUE;
> +av_log(s, AV_LOG_WARNING,
> +   "Discarded timestamp on corrupted packet (stream = %d)\n",
> +   pkt->stream_index);
> +}

how many of the cases that set AV_PKT_FLAG_CORRUPT can even be due to
the timestamp ?

We set this for truncated / EOF cases and all kinds of stuff that
are known to be unrelated to the timestamps


[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 1
"Used only once"- "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/3] lavf/utils: add flag to discard timestamps on corrupted frames

[Carl Eugen Hoyos]

2017-12-08 5:23 GMT+01:00 Rodger Combs :

> +if ((s->flags & AVFMT_FLAG_DISCARD_CORRUPT_TS) &&
> +(pkt->flags & AV_PKT_FLAG_CORRUPT)) {

The brackets look ugly.

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 2/3] lavf/utils: add flag to discard timestamps on corrupted frames

[Rodger Combs]

---
 libavformat/avformat.h  | 1 +
 libavformat/options_table.h | 1 +
 libavformat/utils.c | 8 
 3 files changed, 10 insertions(+)

diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index 4f2798a871..d10d583dff 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -1450,6 +1450,7 @@ typedef struct AVFormatContext {
 #define AVFMT_FLAG_FAST_SEEK   0x8 ///< Enable fast, but inaccurate seeks 
for some formats
 #define AVFMT_FLAG_SHORTEST   0x10 ///< Stop muxing when the shortest 
stream stops.
 #define AVFMT_FLAG_AUTO_BSF   0x20 ///< Add bitstream filters as requested 
by the muxer
+#define AVFMT_FLAG_DISCARD_CORRUPT_TS 0x40 ///< Discard timestamps of 
frames marked corrupt
 
 /**
  * Maximum size of the data read from input for determining
diff --git a/libavformat/options_table.h b/libavformat/options_table.h
index b8fa47c6fd..515574d3e0 100644
--- a/libavformat/options_table.h
+++ b/libavformat/options_table.h
@@ -58,6 +58,7 @@ static const AVOption avformat_options[] = {
 {"bitexact", "do not write random/volatile data", 0, AV_OPT_TYPE_CONST, { .i64 
= AVFMT_FLAG_BITEXACT }, 0, 0, E, "fflags" },
 {"shortest", "stop muxing with the shortest stream", 0, AV_OPT_TYPE_CONST, { 
.i64 = AVFMT_FLAG_SHORTEST }, 0, 0, E, "fflags" },
 {"autobsf", "add needed bsfs automatically", 0, AV_OPT_TYPE_CONST, { .i64 = 
AVFMT_FLAG_AUTO_BSF }, 0, 0, E, "fflags" },
+{"discardcorruptts", "discard timestamps on corrupted frames", 0, 
AV_OPT_TYPE_CONST, { .i64 = AVFMT_FLAG_DISCARD_CORRUPT_TS }, 0, 0, E, "fflags" 
},
 {"analyzeduration", "specify how many microseconds are analyzed to probe the 
input", OFFSET(max_analyze_duration), AV_OPT_TYPE_INT64, {.i64 = 0 }, 0, 
INT64_MAX, D},
 {"cryptokey", "decryption key", OFFSET(key), AV_OPT_TYPE_BINARY, {.dbl = 0}, 
0, 0, D},
 {"indexmem", "max memory used for timestamp index (per stream)", 
OFFSET(max_index_size), AV_OPT_TYPE_INT, {.i64 = 1<<20 }, 0, INT_MAX, D},
diff --git a/libavformat/utils.c b/libavformat/utils.c
index 84e49208b8..98af941e9f 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -873,6 +873,14 @@ int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
 st->cur_dts = wrap_timestamp(st, st->cur_dts);
 }
 
+if ((s->flags & AVFMT_FLAG_DISCARD_CORRUPT_TS) &&
+(pkt->flags & AV_PKT_FLAG_CORRUPT)) {
+pkt->pts = pkt->dts = AV_NOPTS_VALUE;
+av_log(s, AV_LOG_WARNING,
+   "Discarded timestamp on corrupted packet (stream = %d)\n",
+   pkt->stream_index);
+}
+
 pkt->dts = wrap_timestamp(st, pkt->dts);
 pkt->pts = wrap_timestamp(st, pkt->pts);
 
-- 
2.14.3

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel