Re: [FFmpeg-devel] [PATCH 2/2] avformat/movenc: parse h264 packets to build Sync Sample and Recovery Point tables
On Fri, Jul 30, 2021 at 10:31:53AM -0300, James Almer wrote: > On 7/30/2021 8:33 AM, Michael Niedermayer wrote: > > On Thu, Jul 29, 2021 at 03:09:03PM -0300, James Almer wrote: > > > On 7/29/2021 2:58 PM, Michael Niedermayer wrote: > > > > On Tue, Jul 27, 2021 at 10:08:20AM -0300, James Almer wrote: > > > > > Since we can't blindly trust the keyframe flag in packets and assume > > > > > its > > > > > contents are a valid Sync Sample, do some basic bitstream parsing to > > > > > build the > > > > > Sync Sample table in addition to a Random Access Recovery Point table. > > > > > > > > > > Suggested-by: ffm...@fb.com > > > > > Signed-off-by: James Almer > > > > > --- > > > > >libavformat/movenc.c | 125 > > > > > +-- > > > > >libavformat/movenc.h | 1 + > > > > >tests/ref/lavf-fate/h264.mp4 | 6 +- > > > > >3 files changed, 123 insertions(+), 9 deletions(-) > > > > > > > > A. > > > > Will this allow a user to mark a subset of keyframes as random > > > > access points ? > > > > A user may want seeking to preferably happen to a scene start and not a > > > > frame before > > > > > > > > B. > > > > Will this allow a user to mark a damaged keyframe as such ? > > > > A frame might in fact have been a keyframe and maybe the only in the > > > > file > > > > but maybe was damaged so a parser might fail to detect it. > > > > > > This code will ensure all packets with an IDR picture will be listed in > > > the > > > Sync Sample table, and all packets with a Recovery Point SEI message will > > > be > > > listed in the Random Access Recovery Point table. > > > Whatever is signaled in packets (like the keyframe flag) is ignored to > > > prevent creating non-spec compliant output. > > > > The file in case B will be non compliant, it is damaged data, it cannot > > be muxed in a compliant way. If we support muxing damaged data then > > parsing that data as a way to unconditionally override the user is > > problematic. > > If you try to interpret damaged data the result is undefined, the spec > > does not tell you how to interpret it and 2 parsers can disagree > > if a damaged frame is a keyframe. > > If you don't mark a packet as a Sync Sample, even if it contains damaged > data, the file is still compliant. The point here is to avoid filling the > Sync Sample table with bogus entries. IMHO, damaged data is not always a compliant video stream, otherwise in the most extreem example /dev/random would have to be a compliant h264 stream. > > > > > Lets just as 2 examples consider multiple slices some being parsed as IDR > > some as non IDR, so what is that ? or what if some packet reodering or > > duplication causes parts of a IDR and non IDR frame to be mixed. > > where is the IDR frame in that case ? 2 parsers can disagree > > The AVCodecParserContext will tag a packet with an IDR slice as keyframe, > and then disregard anything else it may find. I made the code i wrote here > do the same. If thats the case, we could take the example of a frame lets say it has 100 slices, and in one of these 1 byte is changed so it becomes a IDR slice That is not really a keyframe or sync symple yet it will be marked as one IIUC. Now to continue with this example, which way will it work better X. With this marked not a keyframe or Y. With this marked as keyframe as the parser detects ? Its not containing a single valid IDR slice just 1 of the non IDR slices have been damaged and are misparsed. That will not work well as a sync sample yet as a normal sample the 1% of damage might not be noticed at all as its copied from the last frame > > > still a decoder can with some luck start decoding from such a > > point i would guess. But does the user want this marked as a keyframe ? > > maybe yes, maybe no. I think taking the users only way to set the keyframe > > flag away is a step backward > > Look at MPEG2 parsing and others in this container. It's the same scenario. > Did any of these concerns show up back then? > What I'm doing here for h264 is the same thing we did for those, to ensure > we don't create non spec compliant files because of damaged or mistagged > input, or a careless user. > > Letting the user mark whatever they want as a Sync Sample on a container > where it's explicit what should and should not be such is not good practice. > If you want that, you can use Matroska and other formats where keyframe > tagging is apparently not strict. > > Now, i want to point out that I wrote this parsing code here because my > previous attempt at stopping the AVCodecParserContext from being too liberal > marking things as keyframes was rejected *precisely* because it would let > the user create non compliant output in mp4. And now you don't want this one > either because you want to let the user create non complaint output. > I'm running in circles here and it's getting tiresome. I see the problem and i agree with you that it is a problem that needs a
Re: [FFmpeg-devel] [PATCH] avcodec/argo: Check for end of input in decode_alcd()
On Fri, Jul 30, 2021 at 1:59 PM Michael Niedermayer
wrote:
> On Fri, Jul 30, 2021 at 08:44:03AM +0200, Paul B Mahol wrote:
> > On Fri, Jul 30, 2021 at 12:16 AM Andreas Rheinhardt <
> > andreas.rheinha...@outlook.com> wrote:
> >
> > > Andreas Rheinhardt:
> > > > Michael Niedermayer:
> > > >> Fixes: reading over the end
> > > >> Fixes:
> > >
> 36346/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ARGO_fuzzer-5366943107383296
> > > >>
> > > >> Found-by: continuous fuzzing process
> > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > >> Signed-off-by: Michael Niedermayer
> > > >> ---
> > > >> libavcodec/argo.c | 2 ++
> > > >> 1 file changed, 2 insertions(+)
> > > >>
> > > >> diff --git a/libavcodec/argo.c b/libavcodec/argo.c
> > > >> index bbdb6ae15f..79a44d2583 100644
> > > >> --- a/libavcodec/argo.c
> > > >> +++ b/libavcodec/argo.c
> > > >> @@ -116,6 +116,8 @@ static int decode_alcd(AVCodecContext *avctx,
> > > AVFrame *frame)
> > > >> int index;
> > > >>
> > > >> if (count == 0) {
> > > >> +if (bytestream2_get_bytes_left(gb) < 1)
> > > >> +return AVERROR_INVALIDDATA;
> > > >> codes = bytestream2_get_byteu(&sb);
> > > >> count = 8;
> > > >> }
> > > >>
> > > > Does the following also fix the issue?
> > > >
> > > > diff --git a/libavcodec/argo.c b/libavcodec/argo.c
> > > >
> > > > index bbdb6ae15f..602c042568 100644
> > > >
> > > > --- a/libavcodec/argo.c
> > > >
> > > > +++ b/libavcodec/argo.c
> > > >
> > > > @@ -102,13 +102,14 @@ static int decode_alcd(AVCodecContext *avctx,
> > > > AVFrame *frame)
> > > >
> > > > uint8_t *dst = frame->data[0];
> > > >
> > > > uint8_t codes = 0;
> > > >
> > > > int count = 0;
> > > >
> > > > +int num_codes = ((frame->width + 1) / 2 * (frame->height + 1) /
> 2 +
> > > > 7) >> 3;
> > > >
> > > >
> > > >
> > > > -if (bytestream2_get_bytes_left(gb) < 1024 + (((frame->width /
> 2) *
> > > > (frame->height / 2) + 7) >> 3))
> > > >
> > > > +if (bytestream2_get_bytes_left(gb) < 1024 + num_codes)
> > > >
> > > > return AVERROR_INVALIDDATA;
> > > >
> > > >
> > > >
> > > > bytestream2_skipu(gb, 1024);
> > > >
> > > > sb = *gb;
> > > >
> > > > -bytestream2_skipu(gb, ((frame->width / 2) * (frame->height / 2)
> +
> > > > 7) >> 3);
> > > >
> > > > +bytestream2_skipu(gb, num_codes);
> > > >
> > > >
> > > >
> > > > for (int y = 0; y < frame->height; y += 2) {
> > > >
> > > > for (int x = 0; x < frame->width; x += 2) {
> > > >
> > > As can be seen from the above patch, my guess is that odd dimensions
> are
> > > the cause (because num_codes as above is the number of codes that is
> > > actually read); but my patch would not only change the criterion for
> > > when to error out, but also how much to skip (i.e. where the real data
> > > begins) and this makes me wonder whether we should not error out in
> this
> > > case (and ask for a sample).
> > >
> >
> > I like Andreas solution much more, anway original video files are having
> > even dimensions,
> > so there is nothing to worry.
>
> I did misread the code, andreas patch or a check for even dimensions is
> better
> than what i posted
>
> should i post a patch checking for even dimensions ?
>
Yes
> if yes, which of the cases should be covered by a even check ?
>
just add check for even dimensions at init?
> i assume alcd and avcf at least
>
> Thanks
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Frequently ignored answer#1 FFmpeg bugs should be sent to our bugtracker.
> User
> questions about the command line tools should be sent to the ffmpeg-user
> ML.
> And questions about how to use libav* should be sent to the libav-user ML.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH][libavcodec] Adding sprite support to duck truemotion1
ffmpegandmahanstreamer@e.email:
> Overrides previous patch i submitted regarding this as the old patch I
> submitted had a bug that is fixed in this one.
>
This is not an acceptable commit message: You are supposed to actually
describe the patch here and not the evolution of your patch. Such a
comment belongs immediately below the --- below.
> ---
> libavcodec/truemotion1.c | 206 +--
> 1 file changed, 174 insertions(+), 32 deletions(-)
>
> diff --git a/libavcodec/truemotion1.c b/libavcodec/truemotion1.c
> index 32d8fb4005..b8e8379c74 100644
> --- a/libavcodec/truemotion1.c
> +++ b/libavcodec/truemotion1.c
> @@ -23,10 +23,11 @@
> * @file
> * Duck TrueMotion v1 Video Decoder by
> * Alex Beregszaszi and
> - * Mike Melanson (melan...@pcisys.net)
> + * Mike Melanson (m...@multimedia.cx)
> *
> - * The TrueMotion v1 decoder presently only decodes 16-bit TM1 data and
> - * outputs RGB555 (or RGB565) data. 24-bit TM1 data is not supported yet.
> + * The TrueMotion v1 decoder presently decodes 16-bit/24-bit TM1 data and
> + * outputs RGB555 (or RGB565) data.
> + * ffmpegandmahanstreamer@e.email: added sprite support 7/22/21
> */
>
> #include
> @@ -56,9 +57,15 @@ typedef struct TrueMotion1Context {
>
> int flags;
> int x, y, w, h;
> +
> +int xoffset;
> +int yoffset;
> +int spritew;
> +int spriteh;
>
> uint32_t y_predictor_table[1024];
> uint32_t c_predictor_table[1024];
> +uint32_t a_predictor_table[1024];
> uint32_t fat_y_predictor_table[1024];
> uint32_t fat_c_predictor_table[1024];
>
> @@ -66,6 +73,7 @@ typedef struct TrueMotion1Context {
> int block_type;
> int block_width;
> int block_height;
> +
Spurious change.
>
> int16_t ydt[8];
> int16_t cdt[8];
> @@ -217,7 +225,10 @@ static int make_cdt16_entry(int p1, int p2, int16_t *cdt)
> lo = b + r;
> return (lo + (lo * (1 << 16))) * 2;
> }
> -
> +static int make_adt_entry(int p1, int p2) {
> +int a = p1 + (p2 * 5);
> +return ((a << 16) << 1);
> +}
> static int make_ydt24_entry(int p1, int p2, int16_t *ydt)
> {
> int lo, hi;
> @@ -251,9 +262,12 @@ static void gen_vector_table15(TrueMotion1Context *s,
> const uint8_t *sel_vector_
> make_ydt15_entry(delta_pair >> 4, delta_pair & 0xf, s->ydt);
> s->c_predictor_table[i+j] = 0xfffe &
> make_cdt15_entry(delta_pair >> 4, delta_pair & 0xf, s->cdt);
> +s->a_predictor_table[i+j] = 0xfffe &
> +make_adt_entry(delta_pair >> 4, delta_pair & 0xf);
> }
> s->y_predictor_table[i+(j-1)] |= 1;
> s->c_predictor_table[i+(j-1)] |= 1;
> +s->a_predictor_table[i+(j-1)] |= 1;
> }
> }
>
> @@ -272,9 +286,12 @@ static void gen_vector_table16(TrueMotion1Context *s,
> const uint8_t *sel_vector_
> make_ydt16_entry(delta_pair >> 4, delta_pair & 0xf, s->ydt);
> s->c_predictor_table[i+j] = 0xfffe &
> make_cdt16_entry(delta_pair >> 4, delta_pair & 0xf, s->cdt);
> +s->a_predictor_table[i+j] = 0xfffe &
> +make_adt_entry(delta_pair >> 4, delta_pair & 0xf);
> }
> s->y_predictor_table[i+(j-1)] |= 1;
> s->c_predictor_table[i+(j-1)] |= 1;
> +s->a_predictor_table[i+(j-1)] |= 1;
> }
> }
>
> @@ -360,9 +377,19 @@ static int truemotion1_decode_header(TrueMotion1Context
> *s)
> s->flags = FLAG_KEYFRAME;
>
> if (s->flags & FLAG_SPRITE) {
> -avpriv_request_sample(s->avctx, "Frame with sprite");
> -/* FIXME header.width, height, xoffset and yoffset aren't
> initialized */
> -return AVERROR_PATCHWELCOME;
> +// https://wiki.multimedia.cx/index.php/Duck_TrueMotion_1
> +header.xoffset = AV_RL16(&header_buffer[13]);
> +header.yoffset = AV_RL16(&header_buffer[15]);
> +header.width = AV_RL16(&header_buffer[17]);
> +header.height = AV_RL16(&header_buffer[19]);
> +s->w = header.xsize;
> +s->h = header.ysize;
> +s->spritew = header.width;
> +s->spriteh = header.height;
> +s->xoffset = header.xoffset;
> +s->yoffset = header.yoffset;
> + // avpriv_request_sample(s->avctx, "Frame with sprite");
> + // return AVERROR_PATCHWELCOME;
> } else {
> s->w = header.xsize;
> s->h = header.ysize;
> @@ -427,6 +454,9 @@ static int truemotion1_decode_header(TrueMotion1Context
> *s)
> * 32 pixels; divide width by 4 to obtain the number of change bits and
> * then round up to the nearest byte. */
> s->mb_change_bits_row_size = ((s->avctx->width >> (2 - width_shift)) +
> 7) >> 3;
> +if (s->flags & FLAG_SPRITE) {
> +s->mb_change_bits_row_size = ((s->spritew >> 2) + 3) >> 2;
> +}
>
> if ((header.deltaset != s->last_deltaset) || (header.vectable !=
> s->last_vectable))
>
Re: [FFmpeg-devel] [PATCH 2/2] avformat/movenc: parse h264 packets to build Sync Sample and Recovery Point tables
On Sat, Jul 31, 2021 at 11:41 AM Michael Niedermayer wrote: > Theres also the 2nd situation where all the information the parser > extracts is already known to the lib user and going over the whole > bitstream is wasting cpu cycles. This can be a situation where for > example all input files where just a moment ago created by FFmpeg > and we assume these would then be as compliant as redoing the parsing > would make them > This is however not necessarily the case, because we only have one keyframe field and there are quite varying definitions of what a container would consider a keyframe for their own purposes. This is the entire reason this patch was created in the first place, because different containers have different keyframe semantics, and a single field in AVPacket cannot cleanly represent it. A container therefore determining for its own definition of keyframe how to mark them is the only reliable way currently. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH][libavcodec] Adding sprite support to duck truemotion1
ffmpegandmahanstreamer@e.email (12021-07-31): > In my next patch how about "This patch adds sprite support to the Duck > TrueMotion1 decoder." Have you considered looking at what other people do, especially regular developers? https://git.ffmpeg.org/ffmpeg.git Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH][libavcodec] Adding sprite support to duck truemotion1
B
U
M
P
so i can submit new patches
July 22, 2021 4:27 PM, ffmpegandmahanstreamer@e.email wrote:
> Overrides previous patch i submitted regarding this as the old patch I
> submitted had a bug that is
> fixed in this one.
>
> ---
> libavcodec/truemotion1.c | 206 +--
> 1 file changed, 174 insertions(+), 32 deletions(-)
>
> diff --git a/libavcodec/truemotion1.c b/libavcodec/truemotion1.c
> index 32d8fb4005..b8e8379c74 100644
> --- a/libavcodec/truemotion1.c
> +++ b/libavcodec/truemotion1.c
> @@ -23,10 +23,11 @@
> * @file
> * Duck TrueMotion v1 Video Decoder by
> * Alex Beregszaszi and
> - * Mike Melanson (melan...@pcisys.net)
> + * Mike Melanson (m...@multimedia.cx)
> *
> - * The TrueMotion v1 decoder presently only decodes 16-bit TM1 data and
> - * outputs RGB555 (or RGB565) data. 24-bit TM1 data is not supported yet.
> + * The TrueMotion v1 decoder presently decodes 16-bit/24-bit TM1 data and
> + * outputs RGB555 (or RGB565) data.
> + * ffmpegandmahanstreamer@e.email: added sprite support 7/22/21
> */
>
> #include
> @@ -56,9 +57,15 @@ typedef struct TrueMotion1Context {
>
> int flags;
> int x, y, w, h;
> +
> + int xoffset;
> + int yoffset;
> + int spritew;
> + int spriteh;
>
> uint32_t y_predictor_table[1024];
> uint32_t c_predictor_table[1024];
> + uint32_t a_predictor_table[1024];
> uint32_t fat_y_predictor_table[1024];
> uint32_t fat_c_predictor_table[1024];
>
> @@ -66,6 +73,7 @@ typedef struct TrueMotion1Context {
> int block_type;
> int block_width;
> int block_height;
> +
>
> int16_t ydt[8];
> int16_t cdt[8];
> @@ -217,7 +225,10 @@ static int make_cdt16_entry(int p1, int p2, int16_t *cdt)
> lo = b + r;
> return (lo + (lo * (1 << 16))) * 2;
> }
> -
> +static int make_adt_entry(int p1, int p2) {
> + int a = p1 + (p2 * 5);
> + return ((a << 16) << 1);
> +}
> static int make_ydt24_entry(int p1, int p2, int16_t *ydt)
> {
> int lo, hi;
> @@ -251,9 +262,12 @@ static void gen_vector_table15(TrueMotion1Context *s,
> const uint8_t
> *sel_vector_
> make_ydt15_entry(delta_pair >> 4, delta_pair & 0xf, s->ydt);
> s->c_predictor_table[i+j] = 0xfffe &
> make_cdt15_entry(delta_pair >> 4, delta_pair & 0xf, s->cdt);
> + s->a_predictor_table[i+j] = 0xfffe &
> + make_adt_entry(delta_pair >> 4, delta_pair & 0xf);
> }
> s->y_predictor_table[i+(j-1)] |= 1;
> s->c_predictor_table[i+(j-1)] |= 1;
> + s->a_predictor_table[i+(j-1)] |= 1;
> }
> }
>
> @@ -272,9 +286,12 @@ static void gen_vector_table16(TrueMotion1Context *s,
> const uint8_t
> *sel_vector_
> make_ydt16_entry(delta_pair >> 4, delta_pair & 0xf, s->ydt);
> s->c_predictor_table[i+j] = 0xfffe &
> make_cdt16_entry(delta_pair >> 4, delta_pair & 0xf, s->cdt);
> + s->a_predictor_table[i+j] = 0xfffe &
> + make_adt_entry(delta_pair >> 4, delta_pair & 0xf);
> }
> s->y_predictor_table[i+(j-1)] |= 1;
> s->c_predictor_table[i+(j-1)] |= 1;
> + s->a_predictor_table[i+(j-1)] |= 1;
> }
> }
>
> @@ -360,9 +377,19 @@ static int truemotion1_decode_header(TrueMotion1Context
> *s)
> s->flags = FLAG_KEYFRAME;
>
> if (s->flags & FLAG_SPRITE) {
> - avpriv_request_sample(s->avctx, "Frame with sprite");
> - /* FIXME header.width, height, xoffset and yoffset aren't initialized */
> - return AVERROR_PATCHWELCOME;
> + // https://wiki.multimedia.cx/index.php/Duck_TrueMotion_1
> + header.xoffset = AV_RL16(&header_buffer[13]);
> + header.yoffset = AV_RL16(&header_buffer[15]);
> + header.width = AV_RL16(&header_buffer[17]);
> + header.height = AV_RL16(&header_buffer[19]);
> + s->w = header.xsize;
> + s->h = header.ysize;
> + s->spritew = header.width;
> + s->spriteh = header.height;
> + s->xoffset = header.xoffset;
> + s->yoffset = header.yoffset;
> + // avpriv_request_sample(s->avctx, "Frame with sprite");
> + // return AVERROR_PATCHWELCOME;
> } else {
> s->w = header.xsize;
> s->h = header.ysize;
> @@ -427,6 +454,9 @@ static int truemotion1_decode_header(TrueMotion1Context
> *s)
> * 32 pixels; divide width by 4 to obtain the number of change bits and
> * then round up to the nearest byte. */
> s->mb_change_bits_row_size = ((s->avctx->width >> (2 - width_shift)) + 7) >>
> 3;
> + if (s->flags & FLAG_SPRITE) {
> + s->mb_change_bits_row_size = ((s->spritew >> 2) + 3) >> 2;
> + }
>
> if ((header.deltaset != s->last_deltaset) || (header.vectable !=
> s->last_vectable))
> {
> @@ -441,7 +471,7 @@ static int truemotion1_decode_header(TrueMotion1Context
> *s)
>
> /* set up pointers to the other key data chunks */
> s->mb_change_bits = s->buf + header.header_size;
> - if (s->flags & FLAG_KEYFRAME) {
> + if ((s->flags & FLAG_KEYFRAME) && ((s->flags & FLAG_SPRITE) == 0)) {
> /* no change bits specified for a keyframe; only index bytes */
> s->index_stream = s->mb_change_bits;
> if (s->avctx->width * s->avctx->height / 2048 + header.header_size > s->size)
> @@ -451,6 +481,10 @@ static int truemotion1_decode_header(TrueMotion1Context
> *s)
> s->index_stream = s->mb_change_bits +
> (s->mb_change_bits_row_size * (s->avctx->heigh
Re: [FFmpeg-devel] [PATCH 2/2] avformat/movenc: parse h264 packets to build Sync Sample and Recovery Point tables
On 7/31/2021 6:41 AM, Michael Niedermayer wrote: On Fri, Jul 30, 2021 at 10:31:53AM -0300, James Almer wrote: On 7/30/2021 8:33 AM, Michael Niedermayer wrote: On Thu, Jul 29, 2021 at 03:09:03PM -0300, James Almer wrote: On 7/29/2021 2:58 PM, Michael Niedermayer wrote: On Tue, Jul 27, 2021 at 10:08:20AM -0300, James Almer wrote: Since we can't blindly trust the keyframe flag in packets and assume its contents are a valid Sync Sample, do some basic bitstream parsing to build the Sync Sample table in addition to a Random Access Recovery Point table. Suggested-by: ffm...@fb.com Signed-off-by: James Almer --- libavformat/movenc.c | 125 +-- libavformat/movenc.h | 1 + tests/ref/lavf-fate/h264.mp4 | 6 +- 3 files changed, 123 insertions(+), 9 deletions(-) A. Will this allow a user to mark a subset of keyframes as random access points ? A user may want seeking to preferably happen to a scene start and not a frame before B. Will this allow a user to mark a damaged keyframe as such ? A frame might in fact have been a keyframe and maybe the only in the file but maybe was damaged so a parser might fail to detect it. This code will ensure all packets with an IDR picture will be listed in the Sync Sample table, and all packets with a Recovery Point SEI message will be listed in the Random Access Recovery Point table. Whatever is signaled in packets (like the keyframe flag) is ignored to prevent creating non-spec compliant output. The file in case B will be non compliant, it is damaged data, it cannot be muxed in a compliant way. If we support muxing damaged data then parsing that data as a way to unconditionally override the user is problematic. If you try to interpret damaged data the result is undefined, the spec does not tell you how to interpret it and 2 parsers can disagree if a damaged frame is a keyframe. If you don't mark a packet as a Sync Sample, even if it contains damaged data, the file is still compliant. The point here is to avoid filling the Sync Sample table with bogus entries. IMHO, damaged data is not always a compliant video stream, otherwise in the most extreem example /dev/random would have to be a compliant h264 stream. Lets just as 2 examples consider multiple slices some being parsed as IDR some as non IDR, so what is that ? or what if some packet reodering or duplication causes parts of a IDR and non IDR frame to be mixed. where is the IDR frame in that case ? 2 parsers can disagree The AVCodecParserContext will tag a packet with an IDR slice as keyframe, and then disregard anything else it may find. I made the code i wrote here do the same. If thats the case, we could take the example of a frame lets say it has 100 slices, and in one of these 1 byte is changed so it becomes a IDR slice That is not really a keyframe or sync symple yet it will be marked as one IIUC. Unless it's the first slice NALU out of those 100 in the AU, it will not be marked as a keyframe by the AVCodecParserContext, nor as a Sync Sample by this code. Now to continue with this example, which way will it work better X. With this marked not a keyframe or Y. With this marked as keyframe as the parser detects ? It should not be marked as a Sync Sample. Its not containing a single valid IDR slice just 1 of the non IDR slices have been damaged and are misparsed. That will not work well as a sync sample yet as a normal sample the 1% of damage might not be noticed at all as its copied from the last frame How about i look at every slice NALU in the AU and ensure there's only IDR slices before marking it as a Sync Sample, instead of only looking at the very first slice and decide based on it? Same thing could be done in the AVCodecParserContext, for that matter. still a decoder can with some luck start decoding from such a point i would guess. But does the user want this marked as a keyframe ? maybe yes, maybe no. I think taking the users only way to set the keyframe flag away is a step backward Look at MPEG2 parsing and others in this container. It's the same scenario. Did any of these concerns show up back then? What I'm doing here for h264 is the same thing we did for those, to ensure we don't create non spec compliant files because of damaged or mistagged input, or a careless user. Letting the user mark whatever they want as a Sync Sample on a container where it's explicit what should and should not be such is not good practice. If you want that, you can use Matroska and other formats where keyframe tagging is apparently not strict. Now, i want to point out that I wrote this parsing code here because my previous attempt at stopping the AVCodecParserContext from being too liberal marking things as keyframes was rejected *precisely* because it would let the user create non compliant output in mp4. And now you don't want this one either because you want to let the user create non complaint output. I'm
Re: [FFmpeg-devel] [PATCH 2/2] avformat/movenc: parse h264 packets to build Sync Sample and Recovery Point tables
On Sat, Jul 31, 2021 at 02:40:11PM +0200, Hendrik Leppkes wrote: > On Sat, Jul 31, 2021 at 11:41 AM Michael Niedermayer > wrote: > > Theres also the 2nd situation where all the information the parser > > extracts is already known to the lib user and going over the whole > > bitstream is wasting cpu cycles. This can be a situation where for > > example all input files where just a moment ago created by FFmpeg > > and we assume these would then be as compliant as redoing the parsing > > would make them > > > > This is however not necessarily the case, because we only have one > keyframe field and there are quite varying definitions of what a > container would consider a keyframe for their own purposes. > This is the entire reason this patch was created in the first place, > because different containers have different keyframe semantics, and a > single field in AVPacket cannot cleanly represent it. > agree on all this and thats why i was pushing a bit toward improving the API. > A container therefore determining for its own definition of keyframe > how to mark them is the only reliable way currently. Does it not feel ugly to you that some data can be set by the user through the API and is used, some can be set and is ignored and re-parsed for some container-codec combinations and some cannot be set at all ? What we have is quite inconsistent. I am trying to push a bit towards improving this. Iam not insisting on it. One thing iam suggesting is to extend the API to at least be able to represent all information. If all information can be represented then that allows moving the "parser" out of the muxer and also naturally allows passing information on from user or demuxer or encoder and also allows judging if we are missing something or already have all data. I may be missing something but to me this just seems cleaner than ignoring the user settings completely, reparsing and then whats the next step ? we replace the parser system but do we leave this inside the muxer or move it back out ? If we move it back out its a odd moving around of the code, if its left in the muxer under a new API then its IMHO a bit unwieldy, sometimes redundant and hard to interact with. Iam clearly not understanding something here of the grand plan ... thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Everything should be made as simple as possible, but not simpler. -- Albert Einstein signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Hardware purchase request
On Thu, Jul 29, 2021 at 8:10 PM Michael Niedermayer wrote: > > On Thu, Jul 29, 2021 at 01:07:39AM +0200, Lynne wrote: [...] > > > Fractal Design Ion+ 860 W > > > Fractal Design Define 7 Compact > > > XFX Speedster MERC 319 Radeon RX 6900 XT > > > Samsung 980 PRO 2TB > > > LG UltraGear 27GN950-B > > > All of this comes to just under 3000 euros, using the current prices on > > > geizhals.eu. > > > > > > If 4000 euros is what we agreed, add: > > > AMD Ryzen 9 5900X > > > Corsair Vengeance RGB Pro 32GB 3600Mhz > > > Asus TUF GAMING X570-PLUS (WI-FI) > > > Which adds up to just under 1000 euros, making it 4000 euros. > > > > > > Components have gotten cheaper since I last checked. > > > > > > > Ping (I pinged Stefano 2 weeks ago as well, no response). > > is there something i can do ? > > just trying to make sure we are not stuck in a misunderstanding, as there was > a mail where some "Michaels" comment was requested, but i thought this was > for Michael Schultheiss not me to reply to and indeed he also replied to that > mail. I confirm SPI approved the request and the amount. I just sent another private email, @lynne please check your spam folder in case it went there. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2] avformat/movenc: parse h264 packets to build Sync Sample and Recovery Point tables
On Sat, Jul 31, 2021 at 01:10:27PM -0300, James Almer wrote: > On 7/31/2021 6:41 AM, Michael Niedermayer wrote: > > On Fri, Jul 30, 2021 at 10:31:53AM -0300, James Almer wrote: > > > On 7/30/2021 8:33 AM, Michael Niedermayer wrote: > > > > On Thu, Jul 29, 2021 at 03:09:03PM -0300, James Almer wrote: > > > > > On 7/29/2021 2:58 PM, Michael Niedermayer wrote: > > > > > > On Tue, Jul 27, 2021 at 10:08:20AM -0300, James Almer wrote: > > > > > > > Since we can't blindly trust the keyframe flag in packets and > > > > > > > assume its > > > > > > > contents are a valid Sync Sample, do some basic bitstream parsing > > > > > > > to build the > > > > > > > Sync Sample table in addition to a Random Access Recovery Point > > > > > > > table. > > > > > > > > > > > > > > Suggested-by: ffm...@fb.com > > > > > > > Signed-off-by: James Almer > > > > > > > --- > > > > > > > libavformat/movenc.c | 125 > > > > > > > +-- > > > > > > > libavformat/movenc.h | 1 + > > > > > > > tests/ref/lavf-fate/h264.mp4 | 6 +- > > > > > > > 3 files changed, 123 insertions(+), 9 deletions(-) > > > > > > > > > > > > > > A. > > > > > > Will this allow a user to mark a subset of keyframes as random > > > > > > access points ? > > > > > > A user may want seeking to preferably happen to a scene start and > > > > > > not a > > > > > > frame before > > > > > > > > > > > > > > B. > > > > > > Will this allow a user to mark a damaged keyframe as such ? > > > > > > A frame might in fact have been a keyframe and maybe the only in > > > > > > the file > > > > > > but maybe was damaged so a parser might fail to detect it. > > > > > > > > > > This code will ensure all packets with an IDR picture will be listed > > > > > in the > > > > > Sync Sample table, and all packets with a Recovery Point SEI message > > > > > will be > > > > > listed in the Random Access Recovery Point table. > > > > > Whatever is signaled in packets (like the keyframe flag) is ignored to > > > > > prevent creating non-spec compliant output. > > > > > > > > The file in case B will be non compliant, it is damaged data, it cannot > > > > be muxed in a compliant way. If we support muxing damaged data then > > > > parsing that data as a way to unconditionally override the user is > > > > problematic. > > > > If you try to interpret damaged data the result is undefined, the spec > > > > does not tell you how to interpret it and 2 parsers can disagree > > > > if a damaged frame is a keyframe. > > > > > > If you don't mark a packet as a Sync Sample, even if it contains damaged > > > data, the file is still compliant. The point here is to avoid filling the > > > Sync Sample table with bogus entries. > > > > IMHO, damaged data is not always a compliant video stream, otherwise in the > > most extreem > > example /dev/random would have to be a compliant h264 stream. > > > > > > > > > > > > > > > Lets just as 2 examples consider multiple slices some being parsed as > > > > IDR > > > > some as non IDR, so what is that ? or what if some packet reodering or > > > > duplication causes parts of a IDR and non IDR frame to be mixed. > > > > where is the IDR frame in that case ? 2 parsers can disagree > > > > > > The AVCodecParserContext will tag a packet with an IDR slice as keyframe, > > > and then disregard anything else it may find. I made the code i wrote here > > > do the same. > > > > If thats the case, we could take the example of a frame lets say it has > > 100 slices, and in one of these 1 byte is changed so it becomes a IDR slice > > That is not really a keyframe or sync symple yet it will > > be marked as one IIUC. > > Unless it's the first slice NALU out of those 100 in the AU, it will not be > marked as a keyframe by the AVCodecParserContext, nor as a Sync Sample by > this code. > > > Now to continue with this example, which way will it work better > > X. With this marked not a keyframe or > > Y. With this marked as keyframe as the parser detects ? > > It should not be marked as a Sync Sample. > > > > > Its not containing a single valid IDR slice just 1 of the non IDR slices > > have been damaged and are misparsed. That will not work well as a sync > > sample > > yet as a normal sample the 1% of damage might not be noticed at all as its > > copied > > from the last frame > > How about i look at every slice NALU in the AU and ensure there's only IDR > slices before marking it as a Sync Sample, instead of only looking at the > very first slice and decide based on it? > Same thing could be done in the AVCodecParserContext, for that matter. Then you get a IDR frame wrong if it has a single damaged slice where its parsed as non IDR. What you could do and i do not suggest this actually, just hypothetically is take the majority >50% being IDR as probably IDR. That will probably require extra care when there are 2 fields or frames in one packet The problem is if you unconditionally overreide the u
[FFmpeg-devel] [PATCH 2/3] avcodec/vc1dec: Disable error concealment for *IMAGE
The existing error concealment makes no sense for the image formats, they use transformed source images which is different from keyframe + MC+difference for which the error concealment is designed. Of course feel free to re-enable this if you have a case where it works and improves vissual results Fixes: Timeout Fixes: 36234/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VC1IMAGE_fuzzer-6300306743885824 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer --- libavcodec/vc1dec.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libavcodec/vc1dec.c b/libavcodec/vc1dec.c index 6cd74a09f1..1fb1950ade 100644 --- a/libavcodec/vc1dec.c +++ b/libavcodec/vc1dec.c @@ -1121,7 +1121,9 @@ static int vc1_decode_frame(AVCodecContext *avctx, void *data, ret = AVERROR_INVALIDDATA; goto err; } -if (!v->field_mode) +if ( !v->field_mode +&& avctx->codec_id != AV_CODEC_ID_WMV3IMAGE +&& avctx->codec_id != AV_CODEC_ID_VC1IMAGE) ff_er_frame_end(&s->er); } -- 2.17.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2] avformat/movenc: parse h264 packets to build Sync Sample and Recovery Point tables
On 7/31/2021 2:46 PM, Michael Niedermayer wrote: On Sat, Jul 31, 2021 at 01:10:27PM -0300, James Almer wrote: On 7/31/2021 6:41 AM, Michael Niedermayer wrote: On Fri, Jul 30, 2021 at 10:31:53AM -0300, James Almer wrote: On 7/30/2021 8:33 AM, Michael Niedermayer wrote: On Thu, Jul 29, 2021 at 03:09:03PM -0300, James Almer wrote: On 7/29/2021 2:58 PM, Michael Niedermayer wrote: On Tue, Jul 27, 2021 at 10:08:20AM -0300, James Almer wrote: Since we can't blindly trust the keyframe flag in packets and assume its contents are a valid Sync Sample, do some basic bitstream parsing to build the Sync Sample table in addition to a Random Access Recovery Point table. Suggested-by: ffm...@fb.com Signed-off-by: James Almer --- libavformat/movenc.c | 125 +-- libavformat/movenc.h | 1 + tests/ref/lavf-fate/h264.mp4 | 6 +- 3 files changed, 123 insertions(+), 9 deletions(-) A. Will this allow a user to mark a subset of keyframes as random access points ? A user may want seeking to preferably happen to a scene start and not a frame before B. Will this allow a user to mark a damaged keyframe as such ? A frame might in fact have been a keyframe and maybe the only in the file but maybe was damaged so a parser might fail to detect it. This code will ensure all packets with an IDR picture will be listed in the Sync Sample table, and all packets with a Recovery Point SEI message will be listed in the Random Access Recovery Point table. Whatever is signaled in packets (like the keyframe flag) is ignored to prevent creating non-spec compliant output. The file in case B will be non compliant, it is damaged data, it cannot be muxed in a compliant way. If we support muxing damaged data then parsing that data as a way to unconditionally override the user is problematic. If you try to interpret damaged data the result is undefined, the spec does not tell you how to interpret it and 2 parsers can disagree if a damaged frame is a keyframe. If you don't mark a packet as a Sync Sample, even if it contains damaged data, the file is still compliant. The point here is to avoid filling the Sync Sample table with bogus entries. IMHO, damaged data is not always a compliant video stream, otherwise in the most extreem example /dev/random would have to be a compliant h264 stream. Lets just as 2 examples consider multiple slices some being parsed as IDR some as non IDR, so what is that ? or what if some packet reodering or duplication causes parts of a IDR and non IDR frame to be mixed. where is the IDR frame in that case ? 2 parsers can disagree The AVCodecParserContext will tag a packet with an IDR slice as keyframe, and then disregard anything else it may find. I made the code i wrote here do the same. If thats the case, we could take the example of a frame lets say it has 100 slices, and in one of these 1 byte is changed so it becomes a IDR slice That is not really a keyframe or sync symple yet it will be marked as one IIUC. Unless it's the first slice NALU out of those 100 in the AU, it will not be marked as a keyframe by the AVCodecParserContext, nor as a Sync Sample by this code. Now to continue with this example, which way will it work better X. With this marked not a keyframe or Y. With this marked as keyframe as the parser detects ? It should not be marked as a Sync Sample. Its not containing a single valid IDR slice just 1 of the non IDR slices have been damaged and are misparsed. That will not work well as a sync sample yet as a normal sample the 1% of damage might not be noticed at all as its copied from the last frame How about i look at every slice NALU in the AU and ensure there's only IDR slices before marking it as a Sync Sample, instead of only looking at the very first slice and decide based on it? Same thing could be done in the AVCodecParserContext, for that matter. Then you get a IDR frame wrong if it has a single damaged slice where its parsed as non IDR. Same situation: It does not get added to the Sync Sample table. What you could do and i do not suggest this actually, just hypothetically is take the majority >50% being IDR as probably IDR. That will probably require extra care when there are 2 fields or frames in one packet The problem is if you unconditionally overreide the user you really need to be perfect and this is not easy. More so as being done by default it too should be low overhead not slowing things down still a decoder can with some luck start decoding from such a point i would guess. But does the user want this marked as a keyframe ? maybe yes, maybe no. I think taking the users only way to set the keyframe flag away is a step backward Look at MPEG2 parsing and others in this container. It's the same scenario. Did any of these concerns show up back then? What I'm doing here for h264 is the same thing we did for those, to ensure we don't create non spec compliant files
[FFmpeg-devel] [PATCH 1/3] avcodec/sbrdsp_fixed: Fix negation overflow in sbr_neg_odd_64_c()
Fixes: negation of -2147483648 cannot be represented in type 'int'; cast to an
unsigned type to negate this value to itself
Fixes:
35593/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AAC_FIXED_fuzzer-5182217725804544
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer
---
libavcodec/sbrdsp_fixed.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/sbrdsp_fixed.c b/libavcodec/sbrdsp_fixed.c
index 91fa664c08..43fcc90ae5 100644
--- a/libavcodec/sbrdsp_fixed.c
+++ b/libavcodec/sbrdsp_fixed.c
@@ -87,7 +87,7 @@ static void sbr_neg_odd_64_c(int *x)
{
int i;
for (i = 1; i < 64; i += 2)
-x[i] = -x[i];
+x[i] = -(unsigned)x[i];
}
static void sbr_qmf_pre_shuffle_c(int *z)
--
2.17.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 3/3] avcodec/argo: Check for even dimensions
Fixes: reading over the end
Fixes:
36346/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ARGO_fuzzer-5366943107383296
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer
---
libavcodec/argo.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/libavcodec/argo.c b/libavcodec/argo.c
index bbdb6ae15f..df9aab92a8 100644
--- a/libavcodec/argo.c
+++ b/libavcodec/argo.c
@@ -684,6 +684,11 @@ static av_cold int decode_init(AVCodecContext *avctx)
return AVERROR_PATCHWELCOME;
}
+if (avctx->width % 2 || avctx->height % 2) {
+avpriv_request_sample(s, "Odd dimensions\n");
+return AVERROR_PATCHWELCOME;
+}
+
s->frame = av_frame_alloc();
if (!s->frame)
return AVERROR(ENOMEM);
--
2.17.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH][libavcodec] Adding sprite support to duck truemotion1
July 31, 2021 8:25 AM, "Andreas Rheinhardt"
wrote:
> ffmpegandmahanstreamer@e.email:
>
>> Overrides previous patch i submitted regarding this as the old patch I
>> submitted had a bug that is
>> fixed in this one.
>
> This is not an acceptable commit message: You are supposed to actually
> describe the patch here and not the evolution of your patch. Such a
> comment belongs immediately below the --- below.
>
In my next patch how about "This patch adds sprite support to the Duck
TrueMotion1 decoder."
I thought it was above the three lines not below?
>> ---
>> libavcodec/truemotion1.c | 206 +--
>> 1 file changed, 174 insertions(+), 32 deletions(-)
>>
>> diff --git a/libavcodec/truemotion1.c b/libavcodec/truemotion1.c
>> index 32d8fb4005..b8e8379c74 100644
>> --- a/libavcodec/truemotion1.c
>> +++ b/libavcodec/truemotion1.c
>> @@ -23,10 +23,11 @@
>> * @file
>> * Duck TrueMotion v1 Video Decoder by
>> * Alex Beregszaszi and
>> - * Mike Melanson (melan...@pcisys.net)
>> + * Mike Melanson (m...@multimedia.cx)
>> *
>> - * The TrueMotion v1 decoder presently only decodes 16-bit TM1 data and
>> - * outputs RGB555 (or RGB565) data. 24-bit TM1 data is not supported yet.
>> + * The TrueMotion v1 decoder presently decodes 16-bit/24-bit TM1 data and
>> + * outputs RGB555 (or RGB565) data.
>> + * ffmpegandmahanstreamer@e.email: added sprite support 7/22/21
>> */
>>
>> #include
>> @@ -56,9 +57,15 @@ typedef struct TrueMotion1Context {
>>
>> int flags;
>> int x, y, w, h;
>> +
>> + int xoffset;
>> + int yoffset;
>> + int spritew;
>> + int spriteh;
>>
>> uint32_t y_predictor_table[1024];
>> uint32_t c_predictor_table[1024];
>> + uint32_t a_predictor_table[1024];
>> uint32_t fat_y_predictor_table[1024];
>> uint32_t fat_c_predictor_table[1024];
>>
>> @@ -66,6 +73,7 @@ typedef struct TrueMotion1Context {
>> int block_type;
>> int block_width;
>> int block_height;
>> +
>
> Spurious change.
Must have been mistake on my patch generator. Will fix in updated patch.
>
>> int16_t ydt[8];
>> int16_t cdt[8];
>> @@ -217,7 +225,10 @@ static int make_cdt16_entry(int p1, int p2, int16_t
>> *cdt)
>> lo = b + r;
>> return (lo + (lo * (1 << 16))) * 2;
>> }
>> -
>> +static int make_adt_entry(int p1, int p2) {
>> + int a = p1 + (p2 * 5);
>> + return ((a << 16) << 1);
>> +}
>> static int make_ydt24_entry(int p1, int p2, int16_t *ydt)
>> {
>> int lo, hi;
>> @@ -251,9 +262,12 @@ static void gen_vector_table15(TrueMotion1Context *s,
>> const uint8_t
>> *sel_vector_
>> make_ydt15_entry(delta_pair >> 4, delta_pair & 0xf, s->ydt);
>> s->c_predictor_table[i+j] = 0xfffe &
>> make_cdt15_entry(delta_pair >> 4, delta_pair & 0xf, s->cdt);
>> + s->a_predictor_table[i+j] = 0xfffe &
>> + make_adt_entry(delta_pair >> 4, delta_pair & 0xf);
>> }
>> s->y_predictor_table[i+(j-1)] |= 1;
>> s->c_predictor_table[i+(j-1)] |= 1;
>> + s->a_predictor_table[i+(j-1)] |= 1;
>> }
>> }
>>
>> @@ -272,9 +286,12 @@ static void gen_vector_table16(TrueMotion1Context *s,
>> const uint8_t
>> *sel_vector_
>> make_ydt16_entry(delta_pair >> 4, delta_pair & 0xf, s->ydt);
>> s->c_predictor_table[i+j] = 0xfffe &
>> make_cdt16_entry(delta_pair >> 4, delta_pair & 0xf, s->cdt);
>> + s->a_predictor_table[i+j] = 0xfffe &
>> + make_adt_entry(delta_pair >> 4, delta_pair & 0xf);
>> }
>> s->y_predictor_table[i+(j-1)] |= 1;
>> s->c_predictor_table[i+(j-1)] |= 1;
>> + s->a_predictor_table[i+(j-1)] |= 1;
>> }
>> }
>>
>> @@ -360,9 +377,19 @@ static int truemotion1_decode_header(TrueMotion1Context
>> *s)
>> s->flags = FLAG_KEYFRAME;
>>
>> if (s->flags & FLAG_SPRITE) {
>> - avpriv_request_sample(s->avctx, "Frame with sprite");
>> - /* FIXME header.width, height, xoffset and yoffset aren't initialized */
>> - return AVERROR_PATCHWELCOME;
>> + // https://wiki.multimedia.cx/index.php/Duck_TrueMotion_1
>> + header.xoffset = AV_RL16(&header_buffer[13]);
>> + header.yoffset = AV_RL16(&header_buffer[15]);
>> + header.width = AV_RL16(&header_buffer[17]);
>> + header.height = AV_RL16(&header_buffer[19]);
>> + s->w = header.xsize;
>> + s->h = header.ysize;
>> + s->spritew = header.width;
>> + s->spriteh = header.height;
>> + s->xoffset = header.xoffset;
>> + s->yoffset = header.yoffset;
>> + // avpriv_request_sample(s->avctx, "Frame with sprite");
>> + // return AVERROR_PATCHWELCOME;
>> } else {
>> s->w = header.xsize;
>> s->h = header.ysize;
>> @@ -427,6 +454,9 @@ static int truemotion1_decode_header(TrueMotion1Context
>> *s)
>> * 32 pixels; divide width by 4 to obtain the number of change bits and
>> * then round up to the nearest byte. */
>> s->mb_change_bits_row_size = ((s->avctx->width >> (2 - width_shift)) + 7) >>
>> 3;
>> + if (s->flags & FLAG_SPRITE) {
>> + s->mb_change_bits_row_size = ((s->spritew >> 2) + 3) >> 2;
>> + }
>>
>> if ((header.deltaset != s->last_deltaset) || (header.vectable !=
>> s->last_vectable))
>> {
>> @@ -441,7 +471,7 @@ static int truemotion1_decode_header(TrueMotion1Context
>> *s)
>>
>> /* set up pointers t
Re: [FFmpeg-devel] Hardware purchase request
31 Jul 2021, 18:24 by stefa...@gmail.com: > On Thu, Jul 29, 2021 at 8:10 PM Michael Niedermayer > wrote: > >> >> On Thu, Jul 29, 2021 at 01:07:39AM +0200, Lynne wrote: >> > [...] > >> > > Fractal Design Ion+ 860 W >> > > Fractal Design Define 7 Compact >> > > XFX Speedster MERC 319 Radeon RX 6900 XT >> > > Samsung 980 PRO 2TB >> > > LG UltraGear 27GN950-B >> > > All of this comes to just under 3000 euros, using the current prices on >> > > geizhals.eu. >> > > >> > > If 4000 euros is what we agreed, add: >> > > AMD Ryzen 9 5900X >> > > Corsair Vengeance RGB Pro 32GB 3600Mhz >> > > Asus TUF GAMING X570-PLUS (WI-FI) >> > > Which adds up to just under 1000 euros, making it 4000 euros. >> > > >> > > Components have gotten cheaper since I last checked. >> > > >> > >> > Ping (I pinged Stefano 2 weeks ago as well, no response). >> >> is there something i can do ? >> >> just trying to make sure we are not stuck in a misunderstanding, as there was >> a mail where some "Michaels" comment was requested, but i thought this was >> for Michael Schultheiss not me to reply to and indeed he also replied to that >> mail. >> > > I confirm SPI approved the request and the amount. > I just sent another private email, @lynne please check your spam > folder in case it went there. > I haven't gotten any emails. Did you send it to the correct address? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH][libavcodec] Adding sprite support to duck truemotion1
Hello GEORGE. "avfilter/avf_showfreqs: switch to TX FFT from avutil" "avfilter/af_silenceremove: make window also depends on input sample format" And from Andreas himself: "postproc/postprocess: Remove legacy cruft" are some examples, so i should be good. Some have a two or three sentences while others just have one sentence. How much do you want me to write? There's not much to it. July 31, 2021 10:01 AM, "Nicolas George" wrote: > ffmpegandmahanstreamer@e.email (12021-07-31): > >> In my next patch how about "This patch adds sprite support to the Duck >> TrueMotion1 decoder." > > Have you considered looking at what other people do, especially regular > developers? > > https://git.ffmpeg.org/ffmpeg.git > > Regards, > > -- > Nicolas George > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Hardware purchase request
On Sat, Jul 31, 2021 at 8:02 PM Lynne wrote: > > 31 Jul 2021, 18:24 by stefa...@gmail.com: [...] > >> > Ping (I pinged Stefano 2 weeks ago as well, no response). > >> > >> is there something i can do ? > >> > >> just trying to make sure we are not stuck in a misunderstanding, as there > >> was > >> a mail where some "Michaels" comment was requested, but i thought this was > >> for Michael Schultheiss not me to reply to and indeed he also replied to > >> that > >> mail. > >> > > > > I confirm SPI approved the request and the amount. > > I just sent another private email, @lynne please check your spam > > folder in case it went there. > > > > I haven't gotten any emails. Did you send it to the correct address? I sent it to d...@lynne.ee, please send me a private email (and add Michael to CC). ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avcodec/faxcompr: Check for end of input in cmode == 1 in decode_group3_2d_line()
Fixes: Infinite loop
Fixes:
35591/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_TIFF_fuzzer-4503764022198272
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer
---
libavcodec/faxcompr.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/libavcodec/faxcompr.c b/libavcodec/faxcompr.c
index 44c1f6f6b9..b283831dae 100644
--- a/libavcodec/faxcompr.c
+++ b/libavcodec/faxcompr.c
@@ -283,6 +283,8 @@ static int decode_group3_2d_line(AVCodecContext *avctx,
GetBitContext *gb,
for (k = 0; k < 2; k++) {
run = 0;
for (;;) {
+if (get_bits_left(gb) <= 0)
+return AVERROR_INVALIDDATA;
t = get_vlc2(gb, ccitt_vlc[mode].table, 9, 2);
if (t == -1) {
av_log(avctx, AV_LOG_ERROR, "Incorrect code\n");
--
2.17.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/3] avcodec/aaccoder: Add minimal bias in search_for_ms()
On Tue, Jun 01, 2021 at 09:33:14AM +0200, Michael Niedermayer wrote: > Fixes: floating point division by 0 > Fixes: Ticket8218 > > Signed-off-by: Michael Niedermayer > --- > libavcodec/aaccoder.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The educated differ from the uneducated as much as the living from the dead. -- Aristotle signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 4/4] avformat/wavdec: Use 64bit in new_pos computation
On Tue, Apr 27, 2021 at 09:21:35PM +0200, Michael Niedermayer wrote: > Fixes: signed integer overflow: 129 * 16711680 cannot be represented in type > 'int' > Fixes: > 29102/clusterfuzz-testcase-minimized-ffmpeg_dem_WAV_fuzzer-6742285317439488 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavformat/wavdec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The worst form of inequality is to try to make unequal things equal. -- Aristotle signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 3/4] avformat/sbgdec: Check for overflow in timestamp preparation
On Tue, Apr 27, 2021 at 09:21:34PM +0200, Michael Niedermayer wrote: > Fixes: signed integer overflow: 9223372036854775807 + 864 cannot be > represented in type 'long' > Fixes: > 29102/clusterfuzz-testcase-minimized-ffmpeg_dem_SBG_fuzzer-6731040263634944 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavformat/sbgdec.c | 4 > 1 file changed, 4 insertions(+) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Republics decline into democracies and democracies degenerate into despotisms. -- Aristotle signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/7] avformat/aiffdec: Check for size overflow in header parsing
On Fri, Apr 23, 2021 at 07:50:51PM +0200, Michael Niedermayer wrote: > Fixes: signed integer overflow: 2147483647 + 1 cannot be represented in type > 'int' > Fixes: > 29102/clusterfuzz-testcase-minimized-ffmpeg_dem_AIFF_fuzzer-6723467048255488 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavformat/aiffdec.c | 3 +++ > 1 file changed, 3 insertions(+) will apply patches 2-7 [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Republics decline into democracies and democracies degenerate into despotisms. -- Aristotle signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2] avformat/movenc: parse h264 packets to build Sync Sample and Recovery Point tables
On Sat, Jul 31, 2021 at 02:51:16PM -0300, James Almer wrote: > On 7/31/2021 2:46 PM, Michael Niedermayer wrote: > > On Sat, Jul 31, 2021 at 01:10:27PM -0300, James Almer wrote: > > > On 7/31/2021 6:41 AM, Michael Niedermayer wrote: > > > > On Fri, Jul 30, 2021 at 10:31:53AM -0300, James Almer wrote: > > > > > On 7/30/2021 8:33 AM, Michael Niedermayer wrote: > > > > > > On Thu, Jul 29, 2021 at 03:09:03PM -0300, James Almer wrote: > > > > > > > On 7/29/2021 2:58 PM, Michael Niedermayer wrote: > > > > > > > > On Tue, Jul 27, 2021 at 10:08:20AM -0300, James Almer wrote: > > > > > > > > > Since we can't blindly trust the keyframe flag in packets and > > > > > > > > > assume its > > > > > > > > > contents are a valid Sync Sample, do some basic bitstream > > > > > > > > > parsing to build the > > > > > > > > > Sync Sample table in addition to a Random Access Recovery > > > > > > > > > Point table. > > > > > > > > > > > > > > > > > > Suggested-by: ffm...@fb.com > > > > > > > > > Signed-off-by: James Almer > > > > > > > > > --- > > > > > > > > > libavformat/movenc.c | 125 > > > > > > > > > +-- > > > > > > > > > libavformat/movenc.h | 1 + > > > > > > > > > tests/ref/lavf-fate/h264.mp4 | 6 +- > > > > > > > > > 3 files changed, 123 insertions(+), 9 deletions(-) > > > > > > > > > > > > > > > > > > > > A. > > > > > > > > Will this allow a user to mark a subset of keyframes as random > > > > > > > > access points ? > > > > > > > > A user may want seeking to preferably happen to a scene start > > > > > > > > and not a > > > > > > > > frame before > > > > > > > > > > > > > > > > > > > > B. > > > > > > > > Will this allow a user to mark a damaged keyframe as such ? > > > > > > > > A frame might in fact have been a keyframe and maybe the only > > > > > > > > in the file > > > > > > > > but maybe was damaged so a parser might fail to detect it. > > > > > > > > > > > > > > This code will ensure all packets with an IDR picture will be > > > > > > > listed in the > > > > > > > Sync Sample table, and all packets with a Recovery Point SEI > > > > > > > message will be > > > > > > > listed in the Random Access Recovery Point table. > > > > > > > Whatever is signaled in packets (like the keyframe flag) is > > > > > > > ignored to > > > > > > > prevent creating non-spec compliant output. > > > > > > > > > > > > The file in case B will be non compliant, it is damaged data, it > > > > > > cannot > > > > > > be muxed in a compliant way. If we support muxing damaged data then > > > > > > parsing that data as a way to unconditionally override the user is > > > > > > problematic. > > > > > > If you try to interpret damaged data the result is undefined, the > > > > > > spec > > > > > > does not tell you how to interpret it and 2 parsers can disagree > > > > > > if a damaged frame is a keyframe. > > > > > > > > > > If you don't mark a packet as a Sync Sample, even if it contains > > > > > damaged > > > > > data, the file is still compliant. The point here is to avoid filling > > > > > the > > > > > Sync Sample table with bogus entries. > > > > > > > > IMHO, damaged data is not always a compliant video stream, otherwise in > > > > the most extreem > > > > example /dev/random would have to be a compliant h264 stream. > > > > > > > > > > > > > > > > > > > > > > > > > Lets just as 2 examples consider multiple slices some being parsed > > > > > > as IDR > > > > > > some as non IDR, so what is that ? or what if some packet reodering > > > > > > or > > > > > > duplication causes parts of a IDR and non IDR frame to be mixed. > > > > > > where is the IDR frame in that case ? 2 parsers can disagree > > > > > > > > > > The AVCodecParserContext will tag a packet with an IDR slice as > > > > > keyframe, > > > > > and then disregard anything else it may find. I made the code i wrote > > > > > here > > > > > do the same. > > > > > > > > If thats the case, we could take the example of a frame lets say it has > > > > 100 slices, and in one of these 1 byte is changed so it becomes a IDR > > > > slice > > > > That is not really a keyframe or sync symple yet it will > > > > be marked as one IIUC. > > > > > > Unless it's the first slice NALU out of those 100 in the AU, it will not > > > be > > > marked as a keyframe by the AVCodecParserContext, nor as a Sync Sample by > > > this code. > > > > > > > Now to continue with this example, which way will it work better > > > > X. With this marked not a keyframe or > > > > Y. With this marked as keyframe as the parser detects ? > > > > > > It should not be marked as a Sync Sample. > > > > > > > > > > > > > Its not containing a single valid IDR slice just 1 of the non IDR slices > > > > have been damaged and are misparsed. That will not work well as a sync > > > > sample > > > > yet as a normal sample the 1% of damage might not be noticed at all as > > > > its copied > > > > from the last frame > > > > > >
Re: [FFmpeg-devel] [PATCH 2/2] avformat/movenc: parse h264 packets to build Sync Sample and Recovery Point tables
On 7/31/2021 5:47 PM, Michael Niedermayer wrote: On Sat, Jul 31, 2021 at 02:51:16PM -0300, James Almer wrote: On 7/31/2021 2:46 PM, Michael Niedermayer wrote: On Sat, Jul 31, 2021 at 01:10:27PM -0300, James Almer wrote: On 7/31/2021 6:41 AM, Michael Niedermayer wrote: On Fri, Jul 30, 2021 at 10:31:53AM -0300, James Almer wrote: On 7/30/2021 8:33 AM, Michael Niedermayer wrote: On Thu, Jul 29, 2021 at 03:09:03PM -0300, James Almer wrote: On 7/29/2021 2:58 PM, Michael Niedermayer wrote: On Tue, Jul 27, 2021 at 10:08:20AM -0300, James Almer wrote: Since we can't blindly trust the keyframe flag in packets and assume its contents are a valid Sync Sample, do some basic bitstream parsing to build the Sync Sample table in addition to a Random Access Recovery Point table. Suggested-by: ffm...@fb.com Signed-off-by: James Almer --- libavformat/movenc.c | 125 +-- libavformat/movenc.h | 1 + tests/ref/lavf-fate/h264.mp4 | 6 +- 3 files changed, 123 insertions(+), 9 deletions(-) A. Will this allow a user to mark a subset of keyframes as random access points ? A user may want seeking to preferably happen to a scene start and not a frame before B. Will this allow a user to mark a damaged keyframe as such ? A frame might in fact have been a keyframe and maybe the only in the file but maybe was damaged so a parser might fail to detect it. This code will ensure all packets with an IDR picture will be listed in the Sync Sample table, and all packets with a Recovery Point SEI message will be listed in the Random Access Recovery Point table. Whatever is signaled in packets (like the keyframe flag) is ignored to prevent creating non-spec compliant output. The file in case B will be non compliant, it is damaged data, it cannot be muxed in a compliant way. If we support muxing damaged data then parsing that data as a way to unconditionally override the user is problematic. If you try to interpret damaged data the result is undefined, the spec does not tell you how to interpret it and 2 parsers can disagree if a damaged frame is a keyframe. If you don't mark a packet as a Sync Sample, even if it contains damaged data, the file is still compliant. The point here is to avoid filling the Sync Sample table with bogus entries. IMHO, damaged data is not always a compliant video stream, otherwise in the most extreem example /dev/random would have to be a compliant h264 stream. Lets just as 2 examples consider multiple slices some being parsed as IDR some as non IDR, so what is that ? or what if some packet reodering or duplication causes parts of a IDR and non IDR frame to be mixed. where is the IDR frame in that case ? 2 parsers can disagree The AVCodecParserContext will tag a packet with an IDR slice as keyframe, and then disregard anything else it may find. I made the code i wrote here do the same. If thats the case, we could take the example of a frame lets say it has 100 slices, and in one of these 1 byte is changed so it becomes a IDR slice That is not really a keyframe or sync symple yet it will be marked as one IIUC. Unless it's the first slice NALU out of those 100 in the AU, it will not be marked as a keyframe by the AVCodecParserContext, nor as a Sync Sample by this code. Now to continue with this example, which way will it work better X. With this marked not a keyframe or Y. With this marked as keyframe as the parser detects ? It should not be marked as a Sync Sample. Its not containing a single valid IDR slice just 1 of the non IDR slices have been damaged and are misparsed. That will not work well as a sync sample yet as a normal sample the 1% of damage might not be noticed at all as its copied from the last frame How about i look at every slice NALU in the AU and ensure there's only IDR slices before marking it as a Sync Sample, instead of only looking at the very first slice and decide based on it? Same thing could be done in the AVCodecParserContext, for that matter. Then you get a IDR frame wrong if it has a single damaged slice where its parsed as non IDR. Same situation: It does not get added to the Sync Sample table. and then you cannot seek to the begin of the file anymore We're trying to create spec complaint files as much as possible, not invalid files that some software can then read better. If the first sample is not tagged as Sync because it didn't meet the requirements, then it's how it's meant to be. Remember that files with no Sync Samples at all are still valid. But i already agreed to let the user override the choice made by this code as a last resort, so no point arguing about it. What you could do and i do not suggest this actually, just hypothetically is take the majority >50% being IDR as probably IDR. That will probably require extra care when there are 2 fields or frames in one packet The problem is if you unconditionally overreide the user you real
Re: [FFmpeg-devel] Fwd: Re: [PATCH] mxfdec.c: prefer metadata from Footer
On Fri, 30 Jul 2021, Tomas Härdin wrote: tor 2021-07-29 klockan 22:17 +0200 skrev Marton Balint: On Fri, 23 Jul 2021, emco...@ffastrans.com wrote: > Am 2021-07-04 20:31, schrieb emco...@ffastrans.com: > > Am 2021-07-04 20:28, schrieb emco...@ffastrans.com: > > > Am 2021-07-04 17:28, schrieb Marton Balint: > > > > On Sat, 3 Jul 2021, Tomas Härdin wrote: > > > > > > > > > lör 2021-07-03 klockan 15:13 +0200 skrev emco...@ffastrans.com: > > > > > > Unfortunately the wetransfer link for the fate samples expired, so > > > > > > i thought it might be a good idea to resend it as link to gdrive: > > > > > > > https://drive.google.com/file/d/1yXTdS9RfOsduzg49vBLEshdmIzdaVQfd/view?usp=sharing > > > > > > Also attached the 2 patches: 1 from cus for mxfdec.c and one from > > > > > > myself for the corresponding fate samples. > > > > > > After applying the mxfdec.c patch, fate will pass with the > > > > > > currently existing tests but the files in the zip must be uploaded > > > > > > to the fate suite before applying my corresponding patch of course > > > > > > (otherwise the files don't exist). > > > > > > > > > > > > It would be cool if someone found the time and wants to apply this. > > > > > > > > > > The patch works (except for the samples not being in FATE) > > > > > > > > Actually I found one file where the packetization behaviour changes, > > > > because after the patch a fake index is generated based of the now > > > > recognized duration: > > > > > > > > samples/MXF/freemxf/freeMXF-mxf-dv-1.mxf > > > > > > > > but I guess the file is wrong because clip wrapped UL is used when > > > > the > > > > file seems frame wrapped instead? > > > > > > Nice finding! I can confirm this, it is actually clip wrapped looking > > > at the mxf dump from bmx. These samples are pretty outdated anyway :D > > > Unfortunately i don't have enough insight on the internals yet on the > > > topic about adding/freeing/deleting metadata sets :-( > > > > FRAME Wrapped it is, not clip wrapped :rolleyes: > > ___ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > > Anything left that needs investigation/correction on this one? ...it > looks to be a very nice addition so far. I was waiting for some comments from Tomas, to see if he has a preference, but I guess I will apply in a few days if no further comments are received. I don't have much more to add. We don't have to support every broken file, and this patch beings us more in line with what's correct, as far as I can tell. Ok, thanks, applied my version then. Regards, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avcodec/mpeg12dec: report error when picture type is unknown and err_detect is EXPLODE
Signed-off-by: Marton Balint
---
libavcodec/mpeg12dec.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 858dca660c..129dc1c2aa 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -1529,7 +1529,7 @@ static void
mpeg_decode_quant_matrix_extension(MpegEncContext *s)
load_matrix(s, s->chroma_inter_matrix, NULL, 0);
}
-static void mpeg_decode_picture_coding_extension(Mpeg1Context *s1)
+static int mpeg_decode_picture_coding_extension(Mpeg1Context *s1)
{
MpegEncContext *s = &s1->mpeg_enc_ctx;
@@ -1541,6 +1541,8 @@ static void
mpeg_decode_picture_coding_extension(Mpeg1Context *s1)
if (!s->pict_type && s1->mpeg_enc_ctx_allocated) {
av_log(s->avctx, AV_LOG_ERROR,
"Missing picture start code, guessing missing values\n");
+if (s->avctx->err_recognition & AV_EF_EXPLODE)
+return AVERROR_INVALIDDATA;
if (s->mpeg_f_code[1][0] == 15 && s->mpeg_f_code[1][1] == 15) {
if (s->mpeg_f_code[0][0] == 15 && s->mpeg_f_code[0][1] == 15)
s->pict_type = AV_PICTURE_TYPE_I;
@@ -1586,6 +1588,8 @@ static void
mpeg_decode_picture_coding_extension(Mpeg1Context *s1)
ff_dlog(s->avctx, "alternate_scan=%d\n", s->alternate_scan);
ff_dlog(s->avctx, "frame_pred_frame_dct=%d\n", s->frame_pred_frame_dct);
ff_dlog(s->avctx, "progressive_frame=%d\n", s->progressive_frame);
+
+return 0;
}
static int mpeg_field_start(MpegEncContext *s, const uint8_t *buf, int
buf_size)
@@ -2599,7 +2603,9 @@ static int decode_chunks(AVCodecContext *avctx, AVFrame
*picture,
break;
case 0x8:
if (last_code == PICTURE_START_CODE) {
-mpeg_decode_picture_coding_extension(s);
+int ret = mpeg_decode_picture_coding_extension(s);
+if (ret < 0)
+ return ret;
} else {
av_log(avctx, AV_LOG_ERROR,
"ignoring pic cod ext after %X\n", last_code);
--
2.26.2
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avformat/mxfdec: make MXFMetadataSet part of all metadata sets
The code expects every kind of metadata set to start with the generic metadata
set attributes.
Signed-off-by: Marton Balint
---
libavformat/mxfdec.c | 66 ++--
1 file changed, 21 insertions(+), 45 deletions(-)
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index fd22680adb..34cbd2cd77 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -106,17 +106,19 @@ typedef struct MXFPartition {
KLVPacket first_essence_klv;
} MXFPartition;
-typedef struct MXFCryptoContext {
+typedef struct MXFMetadataSet {
UID uid;
MXFPartition *partition;
enum MXFMetadataSetType type;
+} MXFMetadataSet;
+
+typedef struct MXFCryptoContext {
+MXFMetadataSet meta;
UID source_container_ul;
} MXFCryptoContext;
typedef struct MXFStructuralComponent {
-UID uid;
-MXFPartition *partition;
-enum MXFMetadataSetType type;
+MXFMetadataSet meta;
UID source_package_ul;
UID source_package_uid;
UID data_definition_ul;
@@ -126,9 +128,7 @@ typedef struct MXFStructuralComponent {
} MXFStructuralComponent;
typedef struct MXFSequence {
-UID uid;
-MXFPartition *partition;
-enum MXFMetadataSetType type;
+MXFMetadataSet meta;
UID data_definition_ul;
UID *structural_components_refs;
int structural_components_count;
@@ -137,9 +137,7 @@ typedef struct MXFSequence {
} MXFSequence;
typedef struct MXFTimecodeComponent {
-UID uid;
-MXFPartition *partition;
-enum MXFMetadataSetType type;
+MXFMetadataSet meta;
int drop_frame;
int start_frame;
struct AVRational rate;
@@ -147,33 +145,25 @@ typedef struct MXFTimecodeComponent {
} MXFTimecodeComponent;
typedef struct {
-UID uid;
-MXFPartition *partition;
-enum MXFMetadataSetType type;
+MXFMetadataSet meta;
UID input_segment_ref;
} MXFPulldownComponent;
typedef struct {
-UID uid;
-MXFPartition *partition;
-enum MXFMetadataSetType type;
+MXFMetadataSet meta;
UID *structural_components_refs;
int structural_components_count;
int64_t duration;
} MXFEssenceGroup;
typedef struct {
-UID uid;
-MXFPartition *partition;
-enum MXFMetadataSetType type;
+MXFMetadataSet meta;
char *name;
char *value;
} MXFTaggedValue;
typedef struct {
-UID uid;
-MXFPartition *partition;
-enum MXFMetadataSetType type;
+MXFMetadataSet meta;
MXFSequence *sequence; /* mandatory, and only one */
UID sequence_ref;
int track_id;
@@ -190,9 +180,7 @@ typedef struct {
} MXFTrack;
typedef struct MXFDescriptor {
-UID uid;
-MXFPartition *partition;
-enum MXFMetadataSetType type;
+MXFMetadataSet meta;
UID essence_container_ul;
UID essence_codec_ul;
UID codec_ul;
@@ -230,9 +218,7 @@ typedef struct MXFDescriptor {
} MXFDescriptor;
typedef struct MXFIndexTableSegment {
-UID uid;
-MXFPartition *partition;
-enum MXFMetadataSetType type;
+MXFMetadataSet meta;
int edit_unit_byte_count;
int index_sid;
int body_sid;
@@ -246,9 +232,7 @@ typedef struct MXFIndexTableSegment {
} MXFIndexTableSegment;
typedef struct MXFPackage {
-UID uid;
-MXFPartition *partition;
-enum MXFMetadataSetType type;
+MXFMetadataSet meta;
UID package_uid;
UID package_ul;
UID *tracks_refs;
@@ -261,21 +245,13 @@ typedef struct MXFPackage {
} MXFPackage;
typedef struct MXFEssenceContainerData {
-UID uid;
-MXFPartition *partition;
-enum MXFMetadataSetType type;
+MXFMetadataSet meta;
UID package_uid;
UID package_ul;
int index_sid;
int body_sid;
} MXFEssenceContainerData;
-typedef struct MXFMetadataSet {
-UID uid;
-MXFPartition *partition;
-enum MXFMetadataSetType type;
-} MXFMetadataSet;
-
/* decoded index table */
typedef struct MXFIndexTable {
int index_sid;
@@ -2060,7 +2036,7 @@ static MXFTimecodeComponent*
mxf_resolve_timecode_component(MXFContext *mxf, UID
if (!component)
return NULL;
-switch (component->type) {
+switch (component->meta.type) {
case TimecodeComponent:
return (MXFTimecodeComponent*)component;
case PulldownComponent: /* timcode component may be located on a pulldown
component */
@@ -2096,7 +2072,7 @@ static MXFDescriptor*
mxf_resolve_multidescriptor(MXFContext *mxf, MXFDescriptor
if (!descriptor)
return NULL;
-if (descriptor->type == MultipleDescriptor) {
+if (descriptor->meta.type == MultipleDescriptor) {
for (i = 0; i < descriptor->sub_descriptors_count; i++) {
sub_descriptor = mxf_resolve_strong_ref(mxf,
&descriptor->sub_descriptors_refs[i], Descriptor);
@@ -2108,7 +2084,7 @@ static MXFDescriptor*
mxf_resolve_multidescriptor(MXFContext *mxf, MXFDescriptor
return sub_descriptor;
}
}
-} else if (descriptor->type == Descriptor)
+} else if (descriptor->meta.type
Re: [FFmpeg-devel] [PATCH v3] ffmpeg_opt: restore documented stream selection behaviour
Pushed as c50f5460d2f059e5be393eac90e4eac55a6034c6
On 2021-07-31 09:32, Gyan Doshi wrote:
Plan to push tomorrow.
On 2021-07-29 10:14, Gyan Doshi wrote:
11d3b03fcb added consideration of default stream disposition for audio
and video when choosing the 'best' stream among all the inputs. This can
lead to video streams with lower resolution or audio streams with fewer
channels being selected.
Stream disposition, however, only sets a priority for a stream
among all other streams in the *same input*. It cannot set a priority
for a stream across all inputs.
This patch sets a middle-way and selects the best stream from each file
with default disposition considered. Then it discards disposition weight
and selects best stream as per the original criteria of highest
resolution for video and most channels for audio.
---
fftools/ffmpeg_opt.c | 77
1 file changed, 50 insertions(+), 27 deletions(-)
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 1b43bab9fc..34cc6c4fd3 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -2260,23 +2260,35 @@ static int open_output_file(OptionsContext
*o, const char *filename)
if (!o->video_disable && av_guess_codec(oc->oformat, NULL,
filename, NULL, AVMEDIA_TYPE_VIDEO) != AV_CODEC_ID_NONE) {
int best_score = 0, idx = -1;
int qcr = avformat_query_codec(oc->oformat,
oc->oformat->video_codec, 0);
- for (i = 0; i < nb_input_streams; i++) {
- int score;
- ist = input_streams[i];
- score = ist->st->codecpar->width *
ist->st->codecpar->height
- + 1 * !!(ist->st->event_flags &
AVSTREAM_EVENT_FLAG_NEW_PACKETS)
- + 500*!!(ist->st->disposition &
AV_DISPOSITION_DEFAULT);
- if (ist->user_set_discard == AVDISCARD_ALL)
- continue;
- if((qcr!=MKTAG('A', 'P', 'I', 'C')) &&
(ist->st->disposition & AV_DISPOSITION_ATTACHED_PIC))
- score = 1;
- if (ist->st->codecpar->codec_type ==
AVMEDIA_TYPE_VIDEO &&
- score > best_score) {
- if((qcr==MKTAG('A', 'P', 'I', 'C')) &&
!(ist->st->disposition & AV_DISPOSITION_ATTACHED_PIC))
+ for (j = 0; j < nb_input_files; j++) {
+ InputFile *ifile = input_files[j];
+ int file_best_score = 0, file_best_idx = -1;
+ for (i = 0; i < ifile->nb_streams; i++) {
+ int score;
+ ist = input_streams[ifile->ist_index + i];
+ score = ist->st->codecpar->width *
ist->st->codecpar->height
+ + 1 * !!(ist->st->event_flags
& AVSTREAM_EVENT_FLAG_NEW_PACKETS)
+ + 500*!!(ist->st->disposition &
AV_DISPOSITION_DEFAULT);
+ if (ist->user_set_discard == AVDISCARD_ALL)
continue;
- best_score = score;
- idx = i;
+ if((qcr!=MKTAG('A', 'P', 'I', 'C')) &&
(ist->st->disposition & AV_DISPOSITION_ATTACHED_PIC))
+ score = 1;
+ if (ist->st->codecpar->codec_type ==
AVMEDIA_TYPE_VIDEO &&
+ score > file_best_score) {
+ if((qcr==MKTAG('A', 'P', 'I', 'C')) &&
!(ist->st->disposition & AV_DISPOSITION_ATTACHED_PIC))
+ continue;
+ file_best_score = score;
+ file_best_idx = ifile->ist_index + i;
+ }
}
+ if (file_best_idx >= 0) {
+ if((qcr == MKTAG('A', 'P', 'I', 'C')) ||
!(ist->st->disposition & AV_DISPOSITION_ATTACHED_PIC))
+ file_best_score -=
500*!!(input_streams[file_best_idx]->st->disposition &
AV_DISPOSITION_DEFAULT);
+ if (file_best_score > best_score) {
+ best_score = file_best_score;
+ idx = file_best_idx;
+ }
+ }
}
if (idx >= 0)
new_video_stream(o, oc, idx);
@@ -2285,19 +2297,30 @@ static int open_output_file(OptionsContext
*o, const char *filename)
/* audio: most channels */
if (!o->audio_disable && av_guess_codec(oc->oformat, NULL,
filename, NULL, AVMEDIA_TYPE_AUDIO) != AV_CODEC_ID_NONE) {
int best_score = 0, idx = -1;
- for (i = 0; i < nb_input_streams; i++) {
- int score;
- ist = input_streams[i];
- score = ist->st->codecpar->channels
- + 1 * !!(ist->st->event_flags &
AVSTREAM_EVENT_FLAG_NEW_PACKETS)
- + 500*!!(ist->st->disposition &
AV_DISPOSITION_DEFAULT);
- if (ist->user_set_disca
Re: [FFmpeg-devel] [PATCH v2] libavcodec/libx265: add user data unregistered SEI encoding
On Tuesday, 13 July 2021 11:10:20 PM AEST Derek Buitenhuis wrote: > On 7/12/2021 9:24 AM, Brad Hards wrote: > > MISB ST 0604 and ST 2101 require user data unregistered SEI messages > > (precision timestamps and sensor identifiers) to be included. That > > currently isn't supported for libx265. This patch adds support > > for user data unregistered SEI messages in accordance with > > ISO/IEC 23008-2:2020 Section D.2.7 > > > > The design is based on nvenc, with support finished up at > > 57de80673cb > > --- > > > > libavcodec/libx265.c | 33 + > > 1 file changed, 33 insertions(+) > > Should be OK. Can this please be pushed? Brad ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
