Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/cfhd: use LUT for 9 and 18 codebook decompanding

[Paul B Mahol]

On 8/3/20, Martin Storsjö  wrote:
> On Mon, 3 Aug 2020, Paul B Mahol wrote:
>
>> ffmpeg | branch: master | Paul B Mahol  | Mon Aug  3
>> 09:33:36 2020 +0200| [05e58ce4e29ea47b2e06888c64055aa2f8d3e76c] |
>> committer: Paul B Mahol
>>
>> avcodec/cfhd: use LUT for 9 and 18 codebook decompanding
>>
>> Also fix codebook 9 decompanding, fixing artifact with codebook 9
>> samples. Reused Gagandeep Singh patch.
>>
>>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=05e58ce4e29ea47b2e06888c64055aa2f8d3e76c
>> ---
>>
>> libavcodec/cfhd.c | 35 +--
>> libavcodec/cfhd.h |  2 ++
>> 2 files changed, 27 insertions(+), 10 deletions(-)
>
> This broke the fate-cfhd-3 test.

Yes, it should be fixed now.

>
> // Martin
>
> ___
> 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 V3 1/2] dnn/native: add native support for avg_pool

[Fu, Ting]

> -Original Message-
> From: ffmpeg-devel  On Behalf Of Guo,
> Yejun
> Sent: Monday, August 3, 2020 10:20 AM
> To: FFmpeg development discussions and patches 
> Subject: Re: [FFmpeg-devel] [PATCH V3 1/2] dnn/native: add native support for
> avg_pool
> 
> 
> 
> > -Original Message-
> > From: ffmpeg-devel  On Behalf Of Ting
> > Fu
> > Sent: 2020年7月30日 18:03
> > To: ffmpeg-devel@ffmpeg.org
> > Subject: [FFmpeg-devel] [PATCH V3 1/2] dnn/native: add native support
> > for avg_pool
> >
> > Not support pooling strides in channel dimension now.
> > It can be tested with the model generated with below python script:
> >
> > import tensorflow as tf
> > import numpy as np
> > import imageio
> >
> > in_img = imageio.imread('input_odd.jpg') in_img =
> > in_img.astype(np.float32)/255.0 in_data = in_img[np.newaxis, :]
> >
> > x = tf.placeholder(tf.float32, shape=[1, None, None, 3],
> > name='dnn_in') x_pool = tf.nn.avg_pool(x, ksize=[1,2,2,1],
> > strides=[1,2,2,1], padding='SAME')
> 
> I don't see input/output channel is set in this function parameter, why we 
> need
> them in struct AvgPoolParams?

Hi Yejun,

The in/out_channel in struct AvgPoolParams are for assert() the channel of 
input image is equal to model params.
If read image with  rgba then the native will not process correctly, so I think 
this part is necessary.

> 
[...]
> > +if (avgpool_params->padding_method == SAME) {
> > +height_end = height;
> > +width_end = width;
> > +height_radius = avgpool_params->kernel_size - ((height - 1) %
> > kernel_strides + 1);
> > +width_radius = avgpool_params->kernel_size - ((width - 1) %
> > kernel_strides + 1);
> > +height_radius = height_radius < 0 ? 0 : height_radius >> 1;
> > +width_radius = width_radius < 0 ? 0 : width_radius >> 1;
> > +output_height = ceil(height / (kernel_strides * 1.0));
> > +output_width = ceil(width / (kernel_strides * 1.0));
> > +} else {
> []
> add an assert here, since avg_pool only accepts 'VALID' or 'SAME', while
> padding_method has three enum.
> avassert0(padding_method==VALID)
> 

Sure, will add in patch V4.

> > +height_end = height - avgpool_params->kernel_size + 1;
[...]
> > diff --git a/libavfilter/dnn/dnn_backend_native_layer_avgpool.h
> > b/libavfilter/dnn/dnn_backend_native_layer_avgpool.h
> > new file mode 100644
> > index 00..0b37a8f64b
> > --- /dev/null
> > +++ b/libavfilter/dnn/dnn_backend_native_layer_avgpool.h
> > @@ -0,0 +1,35 @@
> > +/*
> > + * Copyright (c) 2018 Sergey Lavrushkin
> []
> remove name here

Sorry, will modify in next patch version.

Thank you for your review.
Ting FU
> 
> ___
> 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] avcodec/mpegaudiodec_template: Check CRCs for layer1 and layer2

[Lynne]

Aug 3, 2020, 18:31 by mich...@niedermayer.cc:

> This differs from the MPEG specification as the actual real world
> files do compute their CRC over variable areas and not the fixed
> ones listed in the specification. This is also the reason for
> the complexity of this code and the need to perform the CRC
> check for layer2 in the middle of layer2 decoding.
>
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/mpegaudiodec_template.c | 61 +-
>  1 file changed, 44 insertions(+), 17 deletions(-)
>
> diff --git a/libavcodec/mpegaudiodec_template.c 
> b/libavcodec/mpegaudiodec_template.c
> index 3d7e3ba4f2..d84e4f1cb6 100644
> --- a/libavcodec/mpegaudiodec_template.c
> +++ b/libavcodec/mpegaudiodec_template.c
> @@ -89,6 +89,7 @@ typedef struct MPADecodeContext {
>  MPADSPContext mpadsp;
>  AVFloatDSPContext *fdsp;
>  AVFrame *frame;
> +int crc;
>

Make this a uint32_t, its what we always store CRCs as even if they're 8 bits.
Thought it was a flag on first read.



>  } MPADecodeContext;
>  
>  #define HEADER_SIZE 4
> @@ -500,12 +501,43 @@ static void imdct12(INTFLOAT *out, SUINTFLOAT *in)
>  out[11] = in0 + in5;
>  }
>  
> +static int handle_crc(MPADecodeContext *s, int sec_len)
> +{
> +if (s->error_protection && (s->err_recognition & AV_EF_CRCCHECK)) {
> +const uint8_t *buf = s->gb.buffer - HEADER_SIZE;
> +int sec_byte_len  = sec_len >> 3;
> +int sec_rem_bits  = sec_len & 7;
> +const AVCRC *crc_tab = av_crc_get_table(AV_CRC_16_ANSI);
> +uint8_t tmp_buf[4];
> +uint32_t crc_val = av_crc(crc_tab, UINT16_MAX, &buf[2], 2);
> +crc_val = av_crc(crc_tab, crc_val, &buf[6], sec_byte_len);
> +
> +AV_WB32(tmp_buf,
> +((buf[6 + sec_byte_len] & (0xFF00>>sec_rem_bits))<<24) +
> +((unsigned)(s->crc<<16) >> sec_rem_bits));
>

If s->crc is a uint32_t then you don't need to cast it here.
Also argument alignment seems wrong, we align each new line of a function
call/macro with the starting bracket rather than just 4 spaces.
And some spaces in between the shift operators would be nice.


> +crc_val = av_crc(crc_tab, crc_val, tmp_buf, 3);
> +
> +if (crc_val) {
> +av_log(s->avctx, AV_LOG_ERROR, "CRC mismatch %X!\n", crc_val);
>

Good idea, printing the difference was very useful when I was debugging,
we should do that everywhere.


How this works was a little confusing at first but makes sense for me,
so apart from those comments patch LGTM, thanks.
___
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 1/2] lavf/url: add ff_url_decompose().

[Marton Balint]

On Thu, 30 Jul 2020, Nicolas George wrote:


Signed-off-by: Nicolas George 
---
libavformat/tests/url.c | 34 +++
libavformat/url.c   | 74 +
libavformat/url.h   | 41 +++
tests/ref/fate/url  | 45 +
4 files changed, 194 insertions(+)


I chose to keep only pointers to the beginnings despite Marton's
suggestion because I find it plays better with re-constructing the URL
afterwards. The property that each char of the string belongs to one and
only one part is a good invariant, and the delimiting characters are
clearly documented and allow to check if the field is present.


Ok.



Compared with the previous iteration, I added a few macros and the
handling of NULL.


diff --git a/libavformat/tests/url.c b/libavformat/tests/url.c
index 1d961a1b43..e7d259ab7d 100644
--- a/libavformat/tests/url.c
+++ b/libavformat/tests/url.c
@@ -21,6 +21,31 @@
#include "libavformat/url.h"
#include "libavformat/avformat.h"

+static void test_decompose(const char *url)
+{
+URLComponents uc;
+int len, ret;
+
+printf("%s =>\n", url);
+ret = ff_url_decompose(&uc, url, NULL);
+if (ret < 0) {
+printf("  error: %s\n", av_err2str(ret));
+return;
+}
+#define PRINT_COMPONENT(comp) \
+len = uc.url_component_end_##comp - uc.comp; \
+if (len) printf("  "#comp": %.*s\n", len, uc.comp);
+PRINT_COMPONENT(scheme);
+PRINT_COMPONENT(authority);
+PRINT_COMPONENT(userinfo);
+PRINT_COMPONENT(host);
+PRINT_COMPONENT(port);
+PRINT_COMPONENT(path);
+PRINT_COMPONENT(query);
+PRINT_COMPONENT(fragment);
+printf("\n");
+}
+
static void test(const char *base, const char *rel)
{
char buf[200], buf2[200];
@@ -51,6 +76,15 @@ static void test2(const char *url)

int main(void)
{
+printf("Testing ff_url_decompose:\n\n");
+test_decompose("http://user:pass@ffmpeg:8080/dir/file?query#fragment";);
+test_decompose("http://ffmpeg/dir/file";);
+test_decompose("file:///dev/null");
+test_decompose("file:/dev/null");
+test_decompose("http://[::1]/dev/null";);
+test_decompose("http://[::1]:8080/dev/null";);
+test_decompose("//ffmpeg/dev/null");
+
printf("Testing ff_make_absolute_url:\n");
test(NULL, "baz");
test("/foo/bar", "baz");
diff --git a/libavformat/url.c b/libavformat/url.c
index 20463a6674..26aaab4019 100644
--- a/libavformat/url.c
+++ b/libavformat/url.c
@@ -78,6 +78,80 @@ int ff_url_join(char *str, int size, const char *proto,
return strlen(str);
}

+static const char *find_delim(const char *delim, const char *cur, const char 
*end)
+{
+while (cur < end && !strchr(delim, *cur))
+cur++;
+return cur;
+}
+
+int ff_url_decompose(URLComponents *uc, const char *url, const char *end)
+{
+const char *cur, *aend, *p;
+
+if (!url) {
+URLComponents nul = { 0 };
+*uc = nul;


So you are returning NULL pointers here and success at the same time. This 
does not look like a good idea, e.g. checking fields later on involves 
arithmetic on NULL pointers, no? I don't really see it useful that we 
handle NULL url here, we are better off with an assert IMHO.



+return 0;
+}
+if (!end)
+end = url + strlen(url);
+cur = uc->url = url;
+
+/* scheme */
+uc->scheme = cur;
+p = find_delim(":/", cur, end); /* lavf "schemes" can contain options */
+if (*p == ':')
+cur = p + 1;
+
+/* authority */
+uc->authority = cur;
+if (end - cur >= 2 && cur[0] == '/' && cur[1] == '/') {
+cur += 2;
+aend = find_delim("/?#", cur, end);
+
+/* userinfo */
+uc->userinfo = cur;
+p = find_delim("@", cur, aend);
+if (*p == '@')
+cur = p + 1;
+
+/* host */
+uc->host = cur;
+if (*cur == '[') { /* hello IPv6, thanks for using colons! */
+p = find_delim("]", cur, aend);
+if (*p != ']')
+return AVERROR(EINVAL);
+if (p + 1 < aend && p[1] != ':')
+return AVERROR(EINVAL);
+cur = p + 1;


This is the only place where we might return failure. Maybe we could 
convert this to void() function to simplify usage a bit, and either

- assume no port, if it is not paraseable or
- not split host and port, so we don't have to parse IPv6 mess here, 
therefore the error can't happen.


Thanks,
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".

Re: [FFmpeg-devel] [PATCH] avutil/opt: Restore NULL input handling to set_string_video_rate()

[Michael Niedermayer]

On Mon, Aug 03, 2020 at 02:07:36PM +0100, Jack Haughton wrote:
> A NULL check in av_parse_video_rate() would certainly not be a bad idea. It
> wouldn't (on its own) restore the NULL input robustness to
> set_string_video_rate() though, as any error value returned from
> av_parse_video_rate() would result in val being logged using %s, which is
> the whole problem that a500b975 was aiming to solve.
> 
> av_parse_video_rate() is also called from a lot more places (most of which
> already have an explicit NULL check immediately before the call), so
> changing the behaviour of that function by adding a NULL check potentially
> has a lot more knock-on effects than simply restoring the previously
> existing robustness to set_string_video_rate(), which is also in keeping
> with the other av_parse_video_rate() callsites.
> 
> You say that the caller 'should not' pass NULL to set_string_video_rate().
> Absolutely, agreed, but the point of checks is to handle unexpected
> situations. 

> If someone does pass NULL, as of a500b975 the response will be

who is passing NULL in which way ?
this is not a public function

You can achieve such a NULL input by seting up an invalid AVOption
but you dont seem to speak about that. 

please explain what sort of robustness you want to achieve here
Is this about something iam missing or a lib user passing invalidly
setup AVOptions ? (he should fix his code) or some FFmpeg internal
robustness or what else ?


> undefined behaviour, where previously it would have been cleanly handled
> with an explicit error code. I'd contend that this is a bad response.


thx

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

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.


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 0/7 v3] 22.2 channel layout support for AAC decoding

[Jan Ekström]

On Sat, Aug 1, 2020 at 2:07 PM Jan Ekström  wrote:
>
> The decoding parts of this patch set have not changed from v2.
> I have now added changes so that 22.2 gets properly handled as
> 5.1 (for which it has backwards compatibility) until it gets a
> proper remix matrix defined in swresample. So
>  -channel_layout "5.1"` now has been verified to just re-utilize
> the six first channels as-is, and stereo output gets downmixed
> from these known six channels.
>
> Now that we actually have sample(s) for channel_config 13, it was possible
> to see at which various points the AAC decoder fails attempting to parse
> the bit stream.
>
> As the feature `-request_channel_layout 9223372036854775808` 
> (AV_CH_LAYOUT_NATIVE)
> seems to work, the channel order has been validated both visually with
> Audacity, as well as by logging the post-reorder element ordering:
>
> (what specific CPE/SCE/LFE elements mean is documented in the
> `avcodec/aacdectab: add mapping for 22.2` commit):
>
> Setting layout map list after reorder...
> tag 0 = { pos: (null) (3), syn_elem: CPE, elem_id: 1 }
> tag 1 = { pos: FC (4), syn_elem: SCE, elem_id: 0 }
> tag 2 = { pos: LFE (8), syn_elem: LFE, elem_id: 0 }
> tag 3 = { pos: (null) (30), syn_elem: CPE, elem_id: 3 }
> tag 4 = { pos: (null) (c0), syn_elem: CPE, elem_id: 0 }
> tag 5 = { pos: BC (100), syn_elem: SCE, elem_id: 1 }
> tag 6 = { pos: LFE2 (8), syn_elem: LFE, elem_id: 1 }
> tag 7 = { pos: (null) (600), syn_elem: CPE, elem_id: 2 }
> tag 8 = { pos: (null) (5000), syn_elem: CPE, elem_id: 4 }
> tag 9 = { pos: TFC (2000), syn_elem: SCE, elem_id: 2 }
> tag 10 = { pos: TC (800), syn_elem: SCE, elem_id: 3 }
> tag 11 = { pos: (null) (28000), syn_elem: CPE, elem_id: 6 }
> tag 12 = { pos: (null) (30), syn_elem: CPE, elem_id: 5 }
> tag 13 = { pos: TBC (1), syn_elem: SCE, elem_id: 4 }
> tag 14 = { pos: BFC (80), syn_elem: SCE, elem_id: 5 }
> tag 15 = { pos: (null) (140), syn_elem: CPE, elem_id: 7 }
>
> Jan Ekström (7):
>   avutil/channel_layout: add 22.2 layout
>   swresample/matrix: switch internal clean_layout function to not drop
> high bits
>   swresample/rematrix: treat 22.2 as 5.1 (back) when mixing
>   avcodec/mpeg4audio: add newer channel_coding mappings
>   avcodec/aacdectab: add mapping for 22.2
>   avcodec/aacdec_template: mark second LFE element as LFE2
>   avcodec/aacdec_template: add support for 22.2 / channel_config 13
>
>  doc/APIchanges   |  5 ++
>  libavcodec/aacdec_template.c | 98 
>  libavcodec/aacdectab.h   | 23 -
>  libavcodec/mpeg4audio.c  | 17 ++-
>  libavcodec/mpeg4audio.h  |  2 +-
>  libavutil/channel_layout.c   |  6 +++
>  libavutil/channel_layout.h   |  6 +++
>  libavutil/version.h  |  2 +-
>  libswresample/rematrix.c | 12 -
>  9 files changed, 153 insertions(+), 18 deletions(-)
>
> --
> 2.26.2
>

Thanks to everyone for reviews, this has now been applied with the
following small changes:
1. AV_CH_BOTTOM_FRONT_CENTER and AV_CH_BOTTOM_FRONT_LEFT were swapped
to match the 22.2 layout (I seem to have originally followed the way
of how the WaveFormatExtensible values were defined for top channels,
but decided after seeing that another channel layout that has bottom
channels also had center as the first one to just make it match the
22.2 ordering).
2. APIchanges was updated.

The diff for these small changes in the first commit was as follows. I
have verified that this had no effect on the decoded result.
diff --git a/doc/APIchanges b/doc/APIchanges
index d839b2c413..a9bf1afd4c 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,10 +15,10 @@ libavutil: 2017-10-21

 API changes, most recent first:

-2020-06-16 - xx - lavu 56.58.100 - channel_layout.h
+2020-08-04 - xx - lavu 56.58.100 - channel_layout.h
   Add AV_CH_LAYOUT_22POINT2 together with its newly required pieces:
-  AV_CH_TOP_SIDE_LEFT, AV_CH_TOP_SIDE_RIGHT, AV_CH_BOTTOM_FRONT_LEFT,
-  AV_CH_BOTTOM_FRONT_CENTER, AV_CH_BOTTOM_FRONT_RIGHT.
+  AV_CH_TOP_SIDE_LEFT, AV_CH_TOP_SIDE_RIGHT, AV_CH_BOTTOM_FRONT_CENTER,
+  AV_CH_BOTTOM_FRONT_LEFT, AV_CH_BOTTOM_FRONT_RIGHT.

 2020-07-xx - xx - lavu 56.57.100 - cpu.h
   Add AV_CPU_FLAG_MMI and AV_CPU_FLAG_MSA.
diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
index d6bfc74927..ac773a9e63 100644
--- a/libavutil/channel_layout.c
+++ b/libavutil/channel_layout.c
@@ -64,8 +64,8 @@ static const struct channel_name channel_names[] = {
 [35] = { "LFE2",  "low frequency 2"   },
 [36] = { "TSL",   "top side left" },
 [37] = { "TSR",   "top side right"},
-[38] = { "BFL",   "bottom front left" },
-[39] = { "BFC",   "bottom front center"   },
+[38] = { "BFC",   "bottom front center"   },
+[39] = { "BFL",   "bottom front left" },
 [40] = { "BFR",   "bottom front right"},
 };

diff --git a/libavutil/channel_layout.h b/libav

Re: [FFmpeg-devel] [PATCH 1/2] avformat/dv: allow returning damaged audio

[Michael Niedermayer]

On Mon, Aug 03, 2020 at 10:38:21PM +0200, Marton Balint wrote:
> 
> 
> On Sun, 2 Aug 2020, Dave Rice wrote:
> 
> > 
> > 
> > > On Aug 1, 2020, at 5:26 PM, Marton Balint  wrote:
> > > 
> > > 
> > > 
> > > On Sat, 1 Aug 2020, Michael Niedermayer wrote:
> > > 
> > > > On Sat, Aug 01, 2020 at 07:28:53PM +0200, Marton Balint wrote:
> > > > > 
> > > > > 
> > > > > On Sat, 1 Aug 2020, Michael Niedermayer wrote:
> > > > > 
> > > > > > Fixes: Ticket8762
> > > > > > Signed-off-by: Michael Niedermayer 
> > > > > > ---
> > > > > > libavformat/dv.c | 49 
> > > > > > +---
> > > > > > 1 file changed, 42 insertions(+), 7 deletions(-)
> > > > > 
> > > > > If "dv remux loses sync", then the timestamps should be fixed, not
> > > > > additional packets should be generated based on previously read 
> > > > > packet data
> > > > > (which is a fragile approach to begin with, e.g. what if the first 
> > > > > frame is
> > > > > the corrupt one?).
> > > > 
> > > > Ticket8762 is about stream copy, so if no packets are returned for audio
> > > > but are for video and just timestamps are updated this would at least on
> > > > its own probably not work that well.
> > > 
> > > If the timestamps are good, a good player should be able to play it
> > > correctly, even if audio stream is sparse.
> > > 
> > > None of the demuxers generate packets because the timestamps are not
> > > continous, I just don't think it would be consistent if DV suddenly
> > > started to do this. E.g. what if the user wants to drop video with
> > > no audio?
> > 
> > In practice, when dv frames with video and no audio are interleaved
> > within a dv stream that otherwise has both, it is because the playback
> > videotape player of the dv tape is in pause mode or the tape is damaged.
> > These frames most common are filled with only video dif blocks that note
> > concealment (so the image is a copy of a prior image) and the audio
> > source pack metadata is missing, but the paylock of the audio dif blocks
> > are filled with error code so they would decode as silence.
> 
> But if the audio source pack metadata is missing, then how can you determine
> the audio settings? 

> Or the number of samples the errornous frame contains
> (e.g. 1600 v.s 1602)?

some testcase would be useful here where this is done clearly wrong currently


[...]

> Also maybe setting the CORRUPT packet flag should be done in this case?

yes was thinking that too, that should be in the next revision

[]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The greatest way to live with honor in this world is to be what we pretend
to be. -- Socrates


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] [FFmpeg-cvslog] avcodec/cfhd: use LUT for 9 and 18 codebook decompanding

[Martin Storsjö]

On Mon, 3 Aug 2020, Paul B Mahol wrote:


ffmpeg | branch: master | Paul B Mahol  | Mon Aug  3 09:33:36 
2020 +0200| [05e58ce4e29ea47b2e06888c64055aa2f8d3e76c] | committer: Paul B Mahol

avcodec/cfhd: use LUT for 9 and 18 codebook decompanding

Also fix codebook 9 decompanding, fixing artifact with codebook 9
samples. Reused Gagandeep Singh patch.


http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=05e58ce4e29ea47b2e06888c64055aa2f8d3e76c

---

libavcodec/cfhd.c | 35 +--
libavcodec/cfhd.h |  2 ++
2 files changed, 27 insertions(+), 10 deletions(-)


This broke the fate-cfhd-3 test.

// Martin

___
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/3] test: hlsenc: Use unique init/segment file names for the fmp4_ac3 test

[Martin Storsjö]

On Sun, 2 Aug 2020, Martin Storsjö wrote:


On Sunday, August 2, 2020, Andreas Rheinhardt 
wrote:
  Martin Storsjö:
  > Previously, the hls-fmp4 and hls-fmp4_ac3 tests used the same
  file
  > names for init and segment files, which occasionally could
  cause
  > corruption and failed tests, if the input files for both tests
  are
  > generated in parallel, as they could overwrite each other.
  >
  > This happened to work some of the time, as the fmp4_ac3 test
  actually
  > only checked the init segment file (which the fmp4 test case
  never
  > wrote, due to using the incorrect hls_segment_type option) and
  the
  > fmp4 test case always regenerated the input files due to
  mismatched
  > target and file names.
  > ---
  >  tests/fate/hlsenc.mak | 6 +++---
  >  1 file changed, 3 insertions(+), 3 deletions(-)
  >
  > diff --git a/tests/fate/hlsenc.mak b/tests/fate/hlsenc.mak
  > index a57450cd7c..b3e87c0542 100644
  > --- a/tests/fate/hlsenc.mak
  > +++ b/tests/fate/hlsenc.mak
  > @@ -101,13 +101,13 @@ tests/data/hls_fmp4_ac3.m3u8: TAG = GEN
  >  tests/data/hls_fmp4_ac3.m3u8: ffmpeg$(PROGSSUF)$(EXESUF) |
  tests/data
  >       $(M)$(TARGET_EXEC) $(TARGET_PATH)/$< \
  >       -stream_loop 4 -i
  $(SAMPLES)/ac3/monsters_inc_5.1_448_small.ac3 -c copy -map 0 \
  > -     -hls_segment_type fmp4 -hls_fmp4_init_filename now.mp4
  -hls_list_size 0 \
  > -     -hls_time 2 -hls_segment_filename
  "$(TARGET_PATH)/tests/data/hls_fmp4_%d.m4s" \
  > +     -hls_segment_type fmp4 -hls_fmp4_init_filename
  now_ac3.mp4 -hls_list_size 0 \
  > +     -hls_time 2 -hls_segment_filename
  "$(TARGET_PATH)/tests/data/hls_fmp4_ac3_%d.m4s" \
  >       $(TARGET_PATH)/tests/data/hls_fmp4_ac3.m3u8 2>/dev/null
  > 
  >  FATE_HLSENC-$(call ALLYES, HLS_DEMUXER EAC3_DEMUXER) +=
  fate-hls-fmp4_ac3
  >  fate-hls-fmp4_ac3: tests/data/hls_fmp4_ac3.m3u8
  > -fate-hls-fmp4_ac3: CMD = probeaudiostream
  $(TARGET_PATH)/tests/data/now.mp4
  > +fate-hls-fmp4_ac3: CMD = probeaudiostream
  $(TARGET_PATH)/tests/data/now_ac3.mp4
  > 
  >  FATE_SAMPLES_FFMPEG += $(FATE_HLSENC-yes)
  >  fate-hlsenc: $(FATE_HLSENC-yes)
  >
  This test uses ffprobe, yet has no ffprobe dependency. It should
  probably be included in FATE_SAMPLES_FFMPEG_FFPROBE; the other
  tests in
  this file meanwhile all generate their samples themselves, so
  they could
  be in FATE_FFMPEG if I am not mistaken.


Fair enough, I guess I can try to make a patch to fix that aspect.

But that's orthogonal to this patch, which I'm hoping would fix the spurious
failures in fate-hls-fmp4 that I've been running into. Any comments on this
change in itself?


This patch was ok'd on irc by Anton and Jan, so I'll go ahead and push it 
- the requested unrelated change is up for review.


// Martin
___
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 1/2] test: hlsenc: Don't generate test data with -re

[Martin Storsjö]

On Mon, 3 Aug 2020, Michael Niedermayer wrote:


On Sun, Aug 02, 2020 at 11:38:51PM +0300, Martin Storsjö wrote:

This parameter artificially throttled the generation of the test sample
to take 5 seconds.
---
 tests/fate/hlsenc.mak | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/fate/hlsenc.mak b/tests/fate/hlsenc.mak
index dce1f377c7..3c767fd5d9 100644
--- a/tests/fate/hlsenc.mak
+++ b/tests/fate/hlsenc.mak
@@ -88,7 +88,7 @@ fate-hls-list-size: CMD = framecrc -flags +bitexact -i 
$(TARGET_PATH)/tests/data
 tests/data/hls_fmp4.m3u8: TAG = GEN
 tests/data/hls_fmp4.m3u8: ffmpeg$(PROGSSUF)$(EXESUF) | tests/data
$(M)$(TARGET_EXEC) $(TARGET_PATH)/$< \
-   -f lavfi -re -i "aevalsrc=cos(2*PI*t)*sin(2*PI*(440+4*t)*t):d=5" -map 0 
-codec:a mp2fixed \
+   -f lavfi -i "aevalsrc=cos(2*PI*t)*sin(2*PI*(440+4*t)*t):d=5" -map 0 
-codec:a mp2fixed \


is this removing the only test which tests the "-re" flag ?
if so that would reduce test coverage


Well, tehcnically yes, but this test doesn't actually test whether the 
"-re" flag does what it's supposed to or anything like that - it only 
tests that adding the flag doesn't error out and doesn't alter the output. 
And it looks very much unintentional here.


// Martin
___
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 1/2] avformat/dv: allow returning damaged audio

[Marton Balint]

On Sun, 2 Aug 2020, Dave Rice wrote:





On Aug 1, 2020, at 5:26 PM, Marton Balint  wrote:



On Sat, 1 Aug 2020, Michael Niedermayer wrote:


On Sat, Aug 01, 2020 at 07:28:53PM +0200, Marton Balint wrote:



On Sat, 1 Aug 2020, Michael Niedermayer wrote:


Fixes: Ticket8762
Signed-off-by: Michael Niedermayer 
---
libavformat/dv.c | 49 +---
1 file changed, 42 insertions(+), 7 deletions(-)


If "dv remux loses sync", then the timestamps should be fixed, not
additional packets should be generated based on previously read packet data
(which is a fragile approach to begin with, e.g. what if the first frame is
the corrupt one?).


Ticket8762 is about stream copy, so if no packets are returned for audio
but are for video and just timestamps are updated this would at least on
its own probably not work that well.


If the timestamps are good, a good player should be able to play it 
correctly, even if audio stream is sparse.


None of the demuxers generate packets because the timestamps are not 
continous, I just don't think it would be consistent if DV suddenly 
started to do this. E.g. what if the user wants to drop video with no 
audio?


In practice, when dv frames with video and no audio are interleaved 
within a dv stream that otherwise has both, it is because the playback 
videotape player of the dv tape is in pause mode or the tape is damaged. 
These frames most common are filled with only video dif blocks that note 
concealment (so the image is a copy of a prior image) and the audio 
source pack metadata is missing, but the paylock of the audio dif blocks 
are filled with error code so they would decode as silence.


But if the audio source pack metadata is missing, then how can you 
determine the audio settings? Or the number of samples the errornous 
frame contains (e.g. 1600 v.s 1602)?


At least can you assume that audio settings are the same throughout a 
recording? Or DV can have mid-stream audio setting changes?




I did a test of 114 dv tape transfers
61 no difference in demuxing between pass and drop (using this patch)
31 the difference is only in the final frames so the impact of the 
demuxer option would be hard for the user to see, no loss of sync, just video 
with no audio at the end
22 the difference occurs mid-stream, with the drop option the demuxer 
outputs video and audio at different rate when hitting frames with no audio 
source pack, so the output of the demuxer loses sync


about the case of a damaged first frame. Do you have a testcase ?


No, but it can happen, can't it? If the stream starts with no audio for 1 
second you will have 1 second A-V desync, as far as I see, that is why I 
believe fixing the timestamps is the proper fix.


Yes this happens (though it is more rare and didn’t occur in the test 
noted above). In that case, ffmpeg shows no audio at all and I’d have to 
read the stream at later frames using -skip_initial_bytes.


I guess this should be fixed as well, although if there is no way to 
determine the audio settings, this might prove to be difficult. Maybe the 
probing should keep reading the file until an audio source pack is found 
and cache that instead of relying on the last pack? At least that would 
give you more consistent results, irrespective of the order of 
reads/seeks, etc. But this can only work if audio settings are the same 
throughout a file, can we assume that?


Also maybe setting the CORRUPT packet flag should be done in this case?

Regards,
Marton






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 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 1/2] test: hlsenc: Don't generate test data with -re

[Michael Niedermayer]

On Sun, Aug 02, 2020 at 11:38:51PM +0300, Martin Storsjö wrote:
> This parameter artificially throttled the generation of the test sample
> to take 5 seconds.
> ---
>  tests/fate/hlsenc.mak | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/fate/hlsenc.mak b/tests/fate/hlsenc.mak
> index dce1f377c7..3c767fd5d9 100644
> --- a/tests/fate/hlsenc.mak
> +++ b/tests/fate/hlsenc.mak
> @@ -88,7 +88,7 @@ fate-hls-list-size: CMD = framecrc -flags +bitexact -i 
> $(TARGET_PATH)/tests/data
>  tests/data/hls_fmp4.m3u8: TAG = GEN
>  tests/data/hls_fmp4.m3u8: ffmpeg$(PROGSSUF)$(EXESUF) | tests/data
>   $(M)$(TARGET_EXEC) $(TARGET_PATH)/$< \
> - -f lavfi -re -i "aevalsrc=cos(2*PI*t)*sin(2*PI*(440+4*t)*t):d=5" -map 0 
> -codec:a mp2fixed \
> + -f lavfi -i "aevalsrc=cos(2*PI*t)*sin(2*PI*(440+4*t)*t):d=5" -map 0 
> -codec:a mp2fixed \

is this removing the only test which tests the "-re" flag ?
if so that would reduce test coverage

thx

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

What does censorship reveal? It reveals fear. -- Julian Assange


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 v2] [RFC] lavc, lavfmt: add FLIF decoding support

[Kartik K. Khullar]

Wow,
That is such an awesome idea.
Did not know about that offsetof() thing earlier. Definitely going to use
this idea.


> IIRC, the Linux kernel makes heavy use of this structure.
>

That's really cool.

Actually I implemented linked lists and not dynamic arrays because some
elements from somewhere in the middle of the list were to be removed while
processing the transform, and dynamic arrays would just not be suitable for
such
usage because then there will be empty blocks lying around in the array all
over.
So that's why I thought of going with a linked list so that I will keep
deleting nodes
from anywhere I want and no problem would arise.

I will try and implement this structure making the list doubly and then add
a
sentinel node and then process it the same way you just suggested.
It will make deletion of nodes much much simpler since I will just have to
pass the
address of the node of list which has to be deleted and offsetof()
arithmetic will help us
find the whole Object and then we will remove it.
Before I was passing position of the node to be deleted and then the list
was traversed
all over till that node and then it was deleted. Definitely was a very
inefficient way.

Thank You so much.
Kartik.
___
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] avcodec/mpegaudiodec_template: disable CRC checking for layers 1 and 2

[Lynne]

Aug 3, 2020, 15:43 by jamr...@gmail.com:

> Layers 1 and 2 use lengths in bits which are not a multiple of 8,
> and our CRC works on a per-byte basis.
>
> Based on b48397e7b8
>

LGTM, that was a really bad messup on my part.
Thanks.
___
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] avcodec/cfhd: use LUT for 9 and 18 codebook decompanding

[Kieran Kunhya]

On Mon, 3 Aug 2020 at 10:48, Paul B Mahol  wrote:

> Hi,
>
> patch attached.
>
>
Seems fine if tested.

Kieran
___
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] avutil/opt: Restore NULL input handling to set_string_video_rate()

[Jack Haughton]

A NULL check in av_parse_video_rate() would certainly not be a bad idea. It
wouldn't (on its own) restore the NULL input robustness to
set_string_video_rate() though, as any error value returned from
av_parse_video_rate() would result in val being logged using %s, which is
the whole problem that a500b975 was aiming to solve.

av_parse_video_rate() is also called from a lot more places (most of which
already have an explicit NULL check immediately before the call), so
changing the behaviour of that function by adding a NULL check potentially
has a lot more knock-on effects than simply restoring the previously
existing robustness to set_string_video_rate(), which is also in keeping
with the other av_parse_video_rate() callsites.

You say that the caller 'should not' pass NULL to set_string_video_rate().
Absolutely, agreed, but the point of checks is to handle unexpected
situations. If someone does pass NULL, as of a500b975 the response will be
undefined behaviour, where previously it would have been cleanly handled
with an explicit error code. I'd contend that this is a bad response.

Thanks
Jack

On Sun, Aug 2, 2020 at 11:28 PM Michael Niedermayer 
wrote:

> On Sun, Aug 02, 2020 at 08:40:27PM +0100, Jack Haughton wrote:
> > Hello,
> >
> > Apologies for the delay in replying. This patch is not to address a
> > specific problem that currently exists, but rather to restore the
> > robustness to invalid input that previously existed in this function. The
> > motivation for the patch is that we would like to merge a500b975 into our
> > local tree to gain support for gcc 10, but there are concerns about the
> > removal of the null check. av_parse_video_rate() passes its argument
> > directly to strcmp, which would cause undefined behaviour if the argument
> > was NULL.
>
> now it makes it even less sense to me.
> av_parse_video_rate() doesnt support a NULL argument before or after your
> patch, its undefined behaviour either way.
> what you change is a private function using that public function.
>
> Now if you changed the public function to do something specific with a NULL
> argument then that would be more robust maybe but not when its done to a
> single caller which should not pass NULL there
>
> thx
>
>
>
> >
> > Thanks,
> > Jack
> >
> > On Fri, Jul 31, 2020 at 8:31 PM Michael Niedermayer
> 
> > wrote:
> >
> > > Hi
> > >
> > > On Fri, Jul 31, 2020 at 03:53:56PM +0100, Jack Haughton wrote:
> > > > Commit a500b975 removed NULL input handling from this function,
> > > > moving the check higher up the call tree in one branch. However,
> > > > there is another call to set_string_video_rate() which may pass
> > > > NULL, and future users of the function may not be clear that
> > > > a NULL check is required. This patch restores the NULL check to
> > > > avoid these problems.
> > >
> > > Does this affect something else than the seting based on defaults ?
> > > because the defaults should probably be valid values
> > > or if you disagree, iam curious where the default would be
> intentionally
> > > invalid
> > >
> > > thx
> > >
> > > [...]
> > > --
> > > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> > >
> > > Breaking DRM is a little like attempting to break through a door even
> > > though the window is wide open and the only thing in the house is a
> bunch
> > > of things you dont want and which you would get tomorrow for free
> anyway
> > >
> >
> >
> > --
> >
> > Argon Design Ltd., Registration No. 06977690, a Broadcom Inc. company
> >
> > Registered in England with registered office at St John's Innovation
> > Centre, Cowley Road, Cambridge, CB4 0WS
> > ___
> > 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".
>
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> It is what and why we do it that matters, not just one of them.
> ___
> 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".



-- 

Argon Design Ltd., Registration No. 06977690, a Broadcom Inc. company

Registered in England with registered office at St John's Innovation
Centre, Cowley Road, Cambridge, CB4 0WS
___
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] Inconsistent handling of initial_padding and PTS adjustment

[Anton Khirnov]

Quoting Kieran Kunhya (2020-08-03 18:36:16)
> On Mon, 3 Aug 2020 at 08:16, Anton Khirnov  wrote:
> 
> > Quoting Kieran Kunhya (2020-07-27 20:15:17)
> > > On Mon, 27 Jul 2020 at 11:09, Anton Khirnov  wrote:
> > >
> > > > Quoting Kieran Kunhya (2020-07-26 01:51:22)
> > > > > Hi,
> > > > >
> > > > > I notice that some encoders adjust the PTS with initial_padding and
> > some
> > > > > don't.
> > > > > Is this intentional and should we decide that all encoders should do
> > > > this?
> > > >
> > > > Which ones don't?
> > > >
> > >
> > > A few here:
> > >
> > https://github.com/FFmpeg/FFmpeg/search?p=1&q=initial_padding&unscoped_q=initial_padding
> >
> > Sounds a lot like "find them yourself". Presumably you already did that,
> > so might as well list at least an example.
> >
> 
> There's nothing to find. It's easy to see from that page which ones do a
> pts subtraction and which ones don't.
> For example libmp3lame.c and libopusenc.c do not.

Both of those use AudioFrameQueue that does pts adjustment. Just did a
test encode with libmp3lame, the first packet's timestamp is negative.

> > > > I don't think this is a matter of opinion really - if encoder adds
> > > > padding and doesn't adjust the timestamps then the output timestamps
> > are
> > > > just plain wrong.
> > > >
> > >
> > > Well some containers have a flag for it. Right now if you encoded with
> > > libopus into mkv or ts you get the PTS offset as well as the syntax
> > element
> > > written to the bitstream.
> >
> > Then I'd say if a container has a special field for padding then it
> > should also adjust timestamps.
> >
> 
> This makes no sense. Either the container writes the special padding field
> and doesn't adjust timestamps.
> Or it assumes the timestamps are already adjusted and writes a zero padding
> field.
> Writing both is clearly wrong.

That depends on the semantics of the container. E.g. in matroska you are
AFAIU supposed to store the "unadjusted" timestamp (i.e. not taking into
account encoder padding). So if the encoder did adjust the timestamp,
the muxer must adjust it back.
Except the relevant code in our matroska muxer is commented out for no
clear reason.

-- 
Anton Khirnov
___
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] doc/developer: origin of tables should be documented.

[Zhao Zhili]

> On Aug 3, 2020, at 5:54 PM, Nicolas George  wrote:
> 
> Nicolas George (12020-07-31):
>> Something like this would be acceptable:
>> 
>> /* Reverse-engineered by encoding various files with the reference
>>   encoder. */
>> 
>> Reluctance to give this little information is baffling to me.
> 
> Can other developers give their input on this?
> 
> Is it acceptable or too much to require at least short comments like
> these ones before tables of numbers in the source code?
> 
> /* Copied from https://oeis.org/A55 */
> 
> /* Computed with =IF(MOD(A1;2)=0;A1/2;3*A1+1) */
> 
> /* Adjusted to match decoding of samples/$dir/*.xxx with screenshots
>   from the game. */
> 
> They are IMHO necessary to let other people learn from the source code,
> which is the difference between Libre Software and non-Libre Software.
> They may probably be useful for the author themself if they come back
> much later.

Agree. And I like it extends to approximation/non-standard algorithms, although
it'd hard to define "non-standard".

> 
> 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] [FFmpeg-cvslog] mpegaudiodec_template: disable CRC checking for layers 1 and 2

[Michael Niedermayer]

On Mon, Aug 03, 2020 at 10:47:44AM +0200, Lynne wrote:
> Aug 3, 2020, 01:45 by mich...@niedermayer.cc:
> 
> > On Sun, Aug 02, 2020 at 08:52:22PM +, Lynne wrote:
> >
> >> ffmpeg | branch: master | Lynne  | Sun Aug  2 22:45:00 2020 
> >> +0200| [b48397e7b84864f2d4c70361a4c4bed93e826753] | committer: Lynne
> >>
> >> mpegaudiodec_template: disable CRC checking for layers 1 and 2
> >>
> >> Layers 1 and 2 use lengths in bits which are not a multiple of 8,
> >> and our CRC works on a per-byte basis.
> >>
> >> > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=b48397e7b84864f2d4c70361a4c4bed93e826753
> >> ---
> >>
> >>  libavcodec/mpegaudiodec_template.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >
> > Please send patches before pushing changes like everyone else
> > it was just luck that i spoted this change
> >
> > there quite possibly might be people who would want to fix this instead
> > of disabling it.
> >
> > I for example would certainly want to take a look if i knew there was
> > a problem and had a testcase (when i have time)
> >
> > thx
> >
> 
> This actually broke decoding if both crccheck and explode were set and I 
> broke it,
> so I think fixing this quickly first is better. No one else has been 
> interested in CRC checks
> after all and I only noticed this because I always run my audio decoder with 
> those settings.
> I did attempt to fix it myself, but like the commit comment said, it requires 
> feeding the
> CRC bits instead of bytes for some inputs, as that's what the MPEG-1 spec 
> says,
> and that's also what libtwolame seems to generate for 2ch 48khz inputs and 
> 128kbps out.
> 

> If you'd like to try fixing it properly you can generate an mp2 file with 
> error protection via
> -c:a libtwolame -error_protection 1 and decode via ffmpeg -err_detect 
> +crccheck 

just tried and it seems working with my patch


> The CRC is otherwise the same, it still reads 16 bits at the start, skips, 
> and reads N bits,
> its just the number N that changed. ISO 11172-3 Annex B defines the numbers,

I thought so too, but that didnt work


> you can find it at 
> http://read.pudn.com/downloads72/doc/260415/11172/11172_3_ANNEXAB.DOC
> Look for the table "NUMBER OF PROTECTED AUDIO_DATA BITS". You can open the
> doc with any code editor, it isn't a binary word doc, just uses some 
> pseudo-markdown syntax.
> 

> Reading bits for CRC isn't a common case, but I've encountered this before 
> when trying
> to make the AAC-HE decoder check the CRC, so maybe having a function in 
> libavcodec or
> libavutil for that would be a good idea.

yes though in the patch posted ive implemented it by transforming the 
non byte input into a multiple of 8 bits.
Just saying so noone is confused by what the code does ...

That could be moved to a common function or some bitwise CRC loop based
one could be addded

thx

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

The greatest way to live with honor in this world is to be what we pretend
to be. -- Socrates


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".

[FFmpeg-devel] [PATCH] avcodec/mpegaudiodec_template: Check CRCs for layer1 and layer2

[Michael Niedermayer]

This differs from the MPEG specification as the actual real world
files do compute their CRC over variable areas and not the fixed
ones listed in the specification. This is also the reason for
the complexity of this code and the need to perform the CRC
check for layer2 in the middle of layer2 decoding.

Signed-off-by: Michael Niedermayer 
---
 libavcodec/mpegaudiodec_template.c | 61 +-
 1 file changed, 44 insertions(+), 17 deletions(-)

diff --git a/libavcodec/mpegaudiodec_template.c 
b/libavcodec/mpegaudiodec_template.c
index 3d7e3ba4f2..d84e4f1cb6 100644
--- a/libavcodec/mpegaudiodec_template.c
+++ b/libavcodec/mpegaudiodec_template.c
@@ -89,6 +89,7 @@ typedef struct MPADecodeContext {
 MPADSPContext mpadsp;
 AVFloatDSPContext *fdsp;
 AVFrame *frame;
+int crc;
 } MPADecodeContext;
 
 #define HEADER_SIZE 4
@@ -500,12 +501,43 @@ static void imdct12(INTFLOAT *out, SUINTFLOAT *in)
 out[11] = in0 + in5;
 }
 
+static int handle_crc(MPADecodeContext *s, int sec_len)
+{
+if (s->error_protection && (s->err_recognition & AV_EF_CRCCHECK)) {
+const uint8_t *buf = s->gb.buffer - HEADER_SIZE;
+int sec_byte_len  = sec_len >> 3;
+int sec_rem_bits  = sec_len & 7;
+const AVCRC *crc_tab = av_crc_get_table(AV_CRC_16_ANSI);
+uint8_t tmp_buf[4];
+uint32_t crc_val = av_crc(crc_tab, UINT16_MAX, &buf[2], 2);
+crc_val = av_crc(crc_tab, crc_val, &buf[6], sec_byte_len);
+
+AV_WB32(tmp_buf,
+((buf[6 + sec_byte_len] & (0xFF00>>sec_rem_bits))<<24) +
+((unsigned)(s->crc<<16) >> sec_rem_bits));
+
+crc_val = av_crc(crc_tab, crc_val, tmp_buf, 3);
+
+if (crc_val) {
+av_log(s->avctx, AV_LOG_ERROR, "CRC mismatch %X!\n", crc_val);
+if (s->err_recognition & AV_EF_EXPLODE)
+return AVERROR_INVALIDDATA;
+}
+}
+return 0;
+}
+
 /* return the number of decoded frames */
 static int mp_decode_layer1(MPADecodeContext *s)
 {
 int bound, i, v, n, ch, j, mant;
 uint8_t allocation[MPA_MAX_CHANNELS][SBLIMIT];
 uint8_t scale_factors[MPA_MAX_CHANNELS][SBLIMIT];
+int ret;
+
+ret = handle_crc(s, (s->nb_channels == 1) ? 8*16  : 8*32);
+if (ret < 0)
+return ret;
 
 if (s->mode == MPA_JSTEREO)
 bound = (s->mode_ext + 1) * 4;
@@ -575,6 +607,7 @@ static int mp_decode_layer2(MPADecodeContext *s)
 unsigned char scale_code[MPA_MAX_CHANNELS][SBLIMIT];
 unsigned char scale_factors[MPA_MAX_CHANNELS][SBLIMIT][3], *sf;
 int scale, qindex, bits, steps, k, l, m, b;
+int ret;
 
 /* select decoding table */
 table = ff_mpa_l2_select_table(s->bit_rate / 1000, s->nb_channels,
@@ -617,6 +650,10 @@ static int mp_decode_layer2(MPADecodeContext *s)
 }
 }
 
+ret = handle_crc(s, get_bits_count(&s->gb) - 16);
+if (ret < 0)
+return ret;
+
 /* scale factors */
 for (i = 0; i < sblimit; i++) {
 for (ch = 0; ch < s->nb_channels; ch++) {
@@ -1310,13 +1347,16 @@ static int mp_decode_layer3(MPADecodeContext *s)
 int gr, ch, blocksplit_flag, i, j, k, n, bits_pos;
 GranuleDef *g;
 int16_t exponents[576]; //FIXME try INTFLOAT
+int ret;
 
 /* read side info */
 if (s->lsf) {
+ret = handle_crc(s, ((s->nb_channels == 1) ? 8*9  : 8*17));
 main_data_begin = get_bits(&s->gb, 8);
 skip_bits(&s->gb, s->nb_channels);
 nb_granules = 1;
 } else {
+ret = handle_crc(s, ((s->nb_channels == 1) ? 8*17 : 8*32));
 main_data_begin = get_bits(&s->gb, 9);
 if (s->nb_channels == 2)
 skip_bits(&s->gb, 3);
@@ -1328,6 +1368,8 @@ static int mp_decode_layer3(MPADecodeContext *s)
 s->granules[ch][1].scfsi = get_bits(&s->gb, 4);
 }
 }
+if (ret < 0)
+return ret;
 
 for (gr = 0; gr < nb_granules; gr++) {
 for (ch = 0; ch < s->nb_channels; ch++) {
@@ -1565,23 +1607,8 @@ static int mp_decode_frame(MPADecodeContext *s, OUT_INT 
**samples,
 OUT_INT *samples_ptr;
 
 init_get_bits(&s->gb, buf + HEADER_SIZE, (buf_size - HEADER_SIZE) * 8);
-
-if (s->error_protection) {
-uint16_t crc = get_bits(&s->gb, 16);
-if (s->err_recognition & AV_EF_CRCCHECK) {
-const int sec_len = s->lsf ? ((s->nb_channels == 1) ? 9  : 17) :
- ((s->nb_channels == 1) ? 17 : 32);
-const AVCRC *crc_tab = av_crc_get_table(AV_CRC_16_ANSI);
-uint32_t crc_cal = av_crc(crc_tab, UINT16_MAX, &buf[2], 2);
-crc_cal = av_crc(crc_tab, crc_cal, &buf[6], sec_len);
-
-if (av_bswap16(crc) ^ crc_cal) {
-av_log(s->avctx, AV_LOG_ERROR, "CRC mismatch!\n");
-if (s->err_recognition & AV_EF_EXPLODE)
-return AVERROR_INVALIDDATA;
-}
-}
-}
+if (s->error_protection)
+s->crc = get_bits(&s->gb, 16);
 
 switch(s->laye

Re: [FFmpeg-devel] Inconsistent handling of initial_padding and PTS adjustment

[Kieran Kunhya]

On Mon, 3 Aug 2020 at 08:16, Anton Khirnov  wrote:

> Quoting Kieran Kunhya (2020-07-27 20:15:17)
> > On Mon, 27 Jul 2020 at 11:09, Anton Khirnov  wrote:
> >
> > > Quoting Kieran Kunhya (2020-07-26 01:51:22)
> > > > Hi,
> > > >
> > > > I notice that some encoders adjust the PTS with initial_padding and
> some
> > > > don't.
> > > > Is this intentional and should we decide that all encoders should do
> > > this?
> > >
> > > Which ones don't?
> > >
> >
> > A few here:
> >
> https://github.com/FFmpeg/FFmpeg/search?p=1&q=initial_padding&unscoped_q=initial_padding
>
> Sounds a lot like "find them yourself". Presumably you already did that,
> so might as well list at least an example.
>

There's nothing to find. It's easy to see from that page which ones do a
pts subtraction and which ones don't.
For example libmp3lame.c and libopusenc.c do not.


> > > I don't think this is a matter of opinion really - if encoder adds
> > > padding and doesn't adjust the timestamps then the output timestamps
> are
> > > just plain wrong.
> > >
> >
> > Well some containers have a flag for it. Right now if you encoded with
> > libopus into mkv or ts you get the PTS offset as well as the syntax
> element
> > written to the bitstream.
>
> Then I'd say if a container has a special field for padding then it
> should also adjust timestamps.
>

This makes no sense. Either the container writes the special padding field
and doesn't adjust timestamps.
Or it assumes the timestamps are already adjusted and writes a zero padding
field.
Writing both is clearly wrong.

Kieran
___
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] avcodec/mpegaudiodec_template: disable CRC checking for layers 1 and 2

[Michael Niedermayer]

On Mon, Aug 03, 2020 at 10:43:11AM -0300, James Almer wrote:
> Layers 1 and 2 use lengths in bits which are not a multiple of 8,
> and our CRC works on a per-byte basis.
> 
> Based on b48397e7b8
> 
> Signed-off-by: James Almer 
> ---
>  libavcodec/mpegaudiodec_template.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

will post an implementation of CRC checks for layer1 and 2.

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

Any man who breaks a law that conscience tells him is unjust and willingly 
accepts the penalty by staying in jail in order to arouse the conscience of 
the community on the injustice of the law is at that moment expressing the 
very highest respect for law. - Martin Luther King Jr


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] doc/developer: origin of tables should be documented.

[Valentin Schweitzer]

Can other developers give their input on this?


I am not currently a FFmpeg developer, so my opinion might
be less relevant than what others have stated.

I think giving a source for things that are not inherently
obvious, but are part of the code is a good idea. Even if
some tables are well explained in some standards or some
values are in their very nature a "magic number", knowing
which standard the table was taken from, or knowing if a
certain value *is* a magic number gives other devs the
information to research and answer their questions
themselves, with little work for the original author.
Some more documentation, be it about source code or
usage may also decrease the rate of questions on
the mailing lists.

Forcing people to source everything leads to useless
sources, but if it's not clear whether or not a piece
of code needs further explanation, I think verbosity
is the better idea.


___
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] Create and populate AVStream side data packet with contents of ISOBMFF edit list entries

[Derek Buitenhuis]

On 30/07/2020 16:55, Matthew Szatmary wrote:
>> This seems to be really MP4 centric. Did you check if such patch sent out
>> for RFC in 2018 could do it?
>>
>> https://patchwork.ffmpeg.org/project/ffmpeg/patch/1522179841-34881-2-git-send-email-derek.buitenh...@gmail.com/
>>
> I was unaware of this patch, And it does not seem to included into master 
> branch

Yeah, that kind of just... dropped off my radar and a I never sent a v2.

I don't really really have a strong opinion on whether to pick that 2 year old
patch up again, or continue with this current patch from Matthew.

I don't really think it matters, as long as anything pushed isn't too rigid to
extend to use with ordered chapters or other cosmic horrors involving MXF.

- Derek

___
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 v2] [RFC] lavc, lavfmt: add FLIF decoding support

[Nicolas George]

Kartik K. Khullar (12020-08-03):
> > Mixing linked lists and indices is usually a sign you are doing
> > something wrong.
> >
> > Here, I believe you would do better with a doubly linked list and
> > pointers instead of indices.
> >
> > Note: there is a trick with doubly linked lists where you put the next
> > and prev pointers together alone in a struct, and have them point not to
> > the actual elements but to the next/prev struct of the elements. That
> > way, you can have the whole list itself as just another such struct, and
> > not have to make special cases for the ends of the list.
> >
> > If you do not know that trick and want to, I can explain.
> 
> Yes please explain more.

It is the combination of two tricks really.

When you have to write operations on a linked list, you frequently need
to check if you are at the beginning or the end of the list: if at the
beginning, update the list pointer, otherwise update the next pointer of
the prev element. This is annoying, and error prone. Doubly so with
doubly-linked lists.

The trick is to add an extra, special element to the list, to make it
circular. This extra element becomes your entry point to the list, it is
called sentinel node. Then, the first element of the list is
sentinel.next, and if the list is doubly-linked, its last element is
sentinel.prev. To test if you are at the end of the list, you do not
test element->next == NULL but element->next == &sentinel. The list is
considered empty if sentinel.next == &sentinel.

It makes many things simpler, because the list is never really empty, it
always contain the sentinel, and all elements have a valid next (and
prev) pointer. That way, inserting or deleting are done exactly the same
way for all elements, be they in the middle, at the end or at the
beginning: just update the prev and next fields of the neighbors, and if
one of them was the sentinel, it just works.

But there is a drawback: if your list elements are big, so is the
sentinel, but everything except its prev and next pointers is wasted.
This is where the second trick comes in.

  ++++++++
  | field1 |<-.  ,->| field1 |<-.  ,->| field1 |<-.  ,->| field1 |
  | field2 |  |  |  | field2 |  |  |  | field2 |  |  |  | field2 |
  | field3 |  |  |  | field3 |  |  |  | field3 |  |  |  | field3 |
  | next   |-'  | next   |-'  | next   |-'  | next   |
  | prev   |  `-| prev   |  `-| prev   |  `-| prev   |
  | field4 || field4 || field4 || field4 |
  | ...|| ...|| ...|| ...|
  ++++++++


Make the list structure contain only the prev and next fields, nothing
else.

  ++ ++ ++ ++
  | pr,nx  |<--> | pr,nx  |<--> | pr,nx  |<--> | pr,nx  |
  ++ ++ ++ ++

At first glance, that makes the list pretty useless. But you can hang
the actual data around the elements:

  ++++++++
  | field1 || field1 || field1 || field1 |
  | field2 || field2 || field2 || field2 |
  | field3 || field3 || field3 || field3 |
  |[pr,nx] |<-->|[pr,nx] |<-->|[pr,nx] |<-->|[pr,nx] |
  | prev   || prev   || prev   || prev   |
  | field4 || field4 || field4 || field4 |
  | ...|| ...|| ...|| ...|
  ++++++++

If you have a pointer to the list structure, you can convert it into a
pointer to the whole structure with just a little pointer arithmetic
using offsetof(). Something like this (very simplified):

typedef struct List {
List *prev, *next;
} List;
typedef struct Object {
Type field1, field2;
...
List list;
};

next_object = (char *)object->list.next - offsetof(Object, list);

Note that you can have the same object part of several lists by having
several List fields with different names.

And if you do that, the sentinel can be just List, it does not need to
be Object.

I have attached a header I use in another project with macros that
define high-level type-safe list operations around these concepts. If
FFmpeg did use many linked lists, I would propose something similar, but
we usually make do with dynamic arrays.

IIRC, the Linux kernel makes heavy use of this structure.

> My apologies, I should have removed other parts of the quoted code which I
> was not replying to.

No need to apologize for doing like 80% of people here ;) But yes,
trimming replies makes the conversation easier to read.

Regards,

-- 
  Nicolas George
#ifndef LINKEDLIST_H
#define LINKEDLIST_H

#include 

#define DECLARE_LIST_HEAD(LType) \
typedef struct LType LType

Re: [FFmpeg-devel] [PATCH] avcodec/mpegaudiodec_template: disable CRC checking for layers 1 and 2

[James Almer]

On 8/3/2020 10:45 AM, Lynne wrote:
> Aug 3, 2020, 15:43 by jamr...@gmail.com:
> 
>> Layers 1 and 2 use lengths in bits which are not a multiple of 8,
>> and our CRC works on a per-byte basis.
>>
>> Based on b48397e7b8
>>
> 
> LGTM, that was a really bad messup on my part.
> Thanks.

Will apply. Thanks.
___
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] fate: Update the test references for h264-skip-nokey and h264-skip-noparse after b48397e7b84864f2

[James Almer]

On 8/3/2020 10:26 AM, James Almer wrote:
> On 8/3/2020 9:45 AM, Michael Niedermayer wrote:
>> On Mon, Aug 03, 2020 at 09:54:41AM +0300, Martin Storsjö wrote:
>>> ---
>>>  tests/ref/fate/h264-skip-nointra | 86 
>>>  tests/ref/fate/h264-skip-nokey   | 86 
>>>  2 files changed, 86 insertions(+), 86 deletions(-)
>>
>> b48397e7b84864f2d4c70361a4c4bed93e826753 is wrong
>>
>> if you update the checksums this may become more difficult
>> to cleanup. Not sure just a feeling ...
>>
>> Please take a more carefull look at 
>> b48397e7b84864f2d4c70361a4c4bed93e826753
>>
>> it does not disable crc checks for layer 1 & 2 it removes reading the CRC
>> field 
>> try decoding 
>> fate-suite//wtv/law-and-order-partial.wtv
>>
>> it plays fine before but since b48397e7b84864f2d4c70361a4c4bed93e826753
>> its audio is broken
>>
>> Thanks
> 
> Then please revert it. It was pushed without a review or a patch sent to
> the ML, and evidently without even a FATE run.

Just reverted it myself, and also sent a (hopefully) correct version of
the change.
___
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/mpegaudiodec_template: disable CRC checking for layers 1 and 2

[James Almer]

Layers 1 and 2 use lengths in bits which are not a multiple of 8,
and our CRC works on a per-byte basis.

Based on b48397e7b8

Signed-off-by: James Almer 
---
 libavcodec/mpegaudiodec_template.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/mpegaudiodec_template.c 
b/libavcodec/mpegaudiodec_template.c
index 3d7e3ba4f2..3ae73d3c8d 100644
--- a/libavcodec/mpegaudiodec_template.c
+++ b/libavcodec/mpegaudiodec_template.c
@@ -1568,7 +1568,7 @@ static int mp_decode_frame(MPADecodeContext *s, OUT_INT 
**samples,
 
 if (s->error_protection) {
 uint16_t crc = get_bits(&s->gb, 16);
-if (s->err_recognition & AV_EF_CRCCHECK) {
+if (s->err_recognition & AV_EF_CRCCHECK && s->layer == 3) {
 const int sec_len = s->lsf ? ((s->nb_channels == 1) ? 9  : 17) :
  ((s->nb_channels == 1) ? 17 : 32);
 const AVCRC *crc_tab = av_crc_get_table(AV_CRC_16_ANSI);
-- 
2.27.0

___
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] fate: Update the test references for h264-skip-nokey and h264-skip-noparse after b48397e7b84864f2

[James Almer]

On 8/3/2020 9:45 AM, Michael Niedermayer wrote:
> On Mon, Aug 03, 2020 at 09:54:41AM +0300, Martin Storsjö wrote:
>> ---
>>  tests/ref/fate/h264-skip-nointra | 86 
>>  tests/ref/fate/h264-skip-nokey   | 86 
>>  2 files changed, 86 insertions(+), 86 deletions(-)
> 
> b48397e7b84864f2d4c70361a4c4bed93e826753 is wrong
> 
> if you update the checksums this may become more difficult
> to cleanup. Not sure just a feeling ...
> 
> Please take a more carefull look at 
> b48397e7b84864f2d4c70361a4c4bed93e826753
> 
> it does not disable crc checks for layer 1 & 2 it removes reading the CRC
> field 
> try decoding 
> fate-suite//wtv/law-and-order-partial.wtv
> 
> it plays fine before but since b48397e7b84864f2d4c70361a4c4bed93e826753
> its audio is broken
> 
> Thanks

Then please revert it. It was pushed without a review or a patch sent to
the ML, and evidently without even a FATE run.
___
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] fate/aac: add missing bitexact flag to some encoder tests

[James Almer]

On 8/3/2020 7:25 AM, Paul B Mahol wrote:
> On 8/3/20, James Almer  wrote:
>> On 8/2/2020 8:43 PM, James Almer wrote:
>>> Will prevet FATE from breaking once LIBAVCODEC_VERSION_MINOR is bumped to
>>> 100.
>>>
>>> Reported-by: zane
>>> Signed-off-by: James Almer 
>>> ---
>>> If anyone wants to check why adding LIBAVCODEC_IDENT to the bitstream (or
>>> rather,
>>> its length increasing) in either
>>> https://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavcodec/aacenc.c;h=e65b76cd74ab3cf86f23de0528cce115e91350f3;hb=HEAD#l60
>>> or
>>> https://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavcodec/aacenc.c;h=e65b76cd74ab3cf86f23de0528cce115e91350f3;hb=HEAD#l689
>>> results in these tests failing when comparing the decoded output with the
>>> reference file, that would be great, because i don't think it should
>>> happen.
>>>
>>>  libavcodec/version.h |  2 +-
>>>  tests/fate/aac.mak   | 12 ++--
>>>  2 files changed, 7 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/libavcodec/version.h b/libavcodec/version.h
>>> index f66919617a..a3f9f828ee 100644
>>> --- a/libavcodec/version.h
>>> +++ b/libavcodec/version.h
>>> @@ -28,7 +28,7 @@
>>>  #include "libavutil/version.h"
>>>
>>>  #define LIBAVCODEC_VERSION_MAJOR  58
>>> -#define LIBAVCODEC_VERSION_MINOR  99
>>> +#define LIBAVCODEC_VERSION_MINOR 100
>>
>> Err, of course this change is not meant to be in this patch. Forgot to
>> remove it while testing. Did so locally just now.
>>
>>>  #define LIBAVCODEC_VERSION_MICRO 100
>>>
>>>  #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR,
>>> \
>>> diff --git a/tests/fate/aac.mak b/tests/fate/aac.mak
>>> index bd283225c1..4b38d3648c 100644
>>> --- a/tests/fate/aac.mak
>>> +++ b/tests/fate/aac.mak
>>> @@ -150,7 +150,7 @@ FATE_AAC += $(FATE_AAC_CT:%=fate-aac-ct-%)
>>>
>>>  FATE_AAC_ENCODE += fate-aac-aref-encode
>>>  fate-aac-aref-encode: ./tests/data/asynth-44100-2.wav
>>> -fate-aac-aref-encode: CMD = enc_dec_pcm adts wav s16le $(REF) -c:a aac
>>> -aac_is 0 -aac_pns 0 -aac_ms 0 -aac_tns 0 -b:a 512k
>>> +fate-aac-aref-encode: CMD = enc_dec_pcm adts wav s16le $(REF) -c:a aac
>>> -aac_is 0 -aac_pns 0 -aac_ms 0 -aac_tns 0 -b:a 512k -fflags +bitexact
>>> -flags +bitexact
>>>  fate-aac-aref-encode: CMP = stddev
>>>  fate-aac-aref-encode: REF = ./tests/data/asynth-44100-2.wav
>>>  fate-aac-aref-encode: CMP_SHIFT = -4096
>>> @@ -159,7 +159,7 @@ fate-aac-aref-encode: SIZE_TOLERANCE = 2464
>>>  fate-aac-aref-encode: FUZZ = 89
>>>
>>>  FATE_AAC_ENCODE += fate-aac-ln-encode
>>> -fate-aac-ln-encode: CMD = enc_dec_pcm adts wav s16le
>>> $(TARGET_SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav -c:a aac
>>> -aac_is 0 -aac_pns 0 -aac_ms 0 -aac_tns 0 -b:a 512k
>>> +fate-aac-ln-encode: CMD = enc_dec_pcm adts wav s16le
>>> $(TARGET_SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav -c:a aac
>>> -aac_is 0 -aac_pns 0 -aac_ms 0 -aac_tns 0 -b:a 512k -fflags +bitexact
>>> -flags +bitexact
>>>  fate-aac-ln-encode: CMP = stddev
>>>  fate-aac-ln-encode: REF =
>>> $(SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav
>>>  fate-aac-ln-encode: CMP_SHIFT = -4096
>>> @@ -168,7 +168,7 @@ fate-aac-ln-encode: SIZE_TOLERANCE = 3560
>>>  fate-aac-ln-encode: FUZZ = 30
>>>
>>>  FATE_AAC_ENCODE += fate-aac-ln-encode-128k
>>> -fate-aac-ln-encode-128k: CMD = enc_dec_pcm adts wav s16le
>>> $(TARGET_SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav -c:a aac
>>> -aac_is 0 -aac_pns 0 -aac_ms 0 -aac_tns 0 -b:a 128k -cutoff 22050
>>> +fate-aac-ln-encode-128k: CMD = enc_dec_pcm adts wav s16le
>>> $(TARGET_SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav -c:a aac
>>> -aac_is 0 -aac_pns 0 -aac_ms 0 -aac_tns 0 -b:a 128k -cutoff 22050 -fflags
>>> +bitexact -flags +bitexact
>>>  fate-aac-ln-encode-128k: CMP = stddev
>>>  fate-aac-ln-encode-128k: REF =
>>> $(SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav
>>>  fate-aac-ln-encode-128k: CMP_SHIFT = -4096
>>> @@ -195,7 +195,7 @@ fate-aac-tns-encode: FUZZ = 7
>>>  fate-aac-tns-encode: SIZE_TOLERANCE = 3560
>>>
>>>  FATE_AAC_ENCODE += fate-aac-is-encode
>>> -fate-aac-is-encode: CMD = enc_dec_pcm adts wav s16le
>>> $(TARGET_SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav -c:a aac
>>> -aac_pns 0 -aac_is 1 -aac_ms 0 -b:a 128k -aac_tns 0 -cutoff 22050
>>> +fate-aac-is-encode: CMD = enc_dec_pcm adts wav s16le
>>> $(TARGET_SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav -c:a aac
>>> -aac_pns 0 -aac_is 1 -aac_ms 0 -b:a 128k -aac_tns 0 -cutoff 22050 -fflags
>>> +bitexact -flags +bitexact
>>>  fate-aac-is-encode: CMP = stddev
>>>  fate-aac-is-encode: REF =
>>> $(SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav
>>>  fate-aac-is-encode: CMP_SHIFT = -4096
>>> @@ -204,7 +204,7 @@ fate-aac-is-encode: SIZE_TOLERANCE = 3560
>>>  fate-aac-is-encode: FUZZ = 10
>>>
>>>  FATE_AAC_ENCODE += fate-aac-ms-encode
>>> -fate-aac-ms-encode: CMD = enc_dec_pcm adts wav s16le
>>> $(TARGET_SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav -c:a aac
>>> -aac_pns 0 -aac_is 0 -aac_ms 1 -aac_tns 0 -b:a

Re: [FFmpeg-devel] [FFmpeg-cvslog] mpegaudiodec_template: disable CRC checking for layers 1 and 2

[Lynne]

Aug 3, 2020, 01:45 by mich...@niedermayer.cc:

> On Sun, Aug 02, 2020 at 08:52:22PM +, Lynne wrote:
>
>> ffmpeg | branch: master | Lynne  | Sun Aug  2 22:45:00 2020 
>> +0200| [b48397e7b84864f2d4c70361a4c4bed93e826753] | committer: Lynne
>>
>> mpegaudiodec_template: disable CRC checking for layers 1 and 2
>>
>> Layers 1 and 2 use lengths in bits which are not a multiple of 8,
>> and our CRC works on a per-byte basis.
>>
>> > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=b48397e7b84864f2d4c70361a4c4bed93e826753
>> ---
>>
>>  libavcodec/mpegaudiodec_template.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>
> Please send patches before pushing changes like everyone else
> it was just luck that i spoted this change
>
> there quite possibly might be people who would want to fix this instead
> of disabling it.
>
> I for example would certainly want to take a look if i knew there was
> a problem and had a testcase (when i have time)
>
> thx
>

This actually broke decoding if both crccheck and explode were set and I broke 
it,
so I think fixing this quickly first is better. No one else has been interested 
in CRC checks
after all and I only noticed this because I always run my audio decoder with 
those settings.
I did attempt to fix it myself, but like the commit comment said, it requires 
feeding the
CRC bits instead of bytes for some inputs, as that's what the MPEG-1 spec says,
and that's also what libtwolame seems to generate for 2ch 48khz inputs and 
128kbps out.

If you'd like to try fixing it properly you can generate an mp2 file with error 
protection via
-c:a libtwolame -error_protection 1 and decode via ffmpeg -err_detect +crccheck 

The CRC is otherwise the same, it still reads 16 bits at the start, skips, and 
reads N bits,
its just the number N that changed. ISO 11172-3 Annex B defines the numbers,
you can find it at 
http://read.pudn.com/downloads72/doc/260415/11172/11172_3_ANNEXAB.DOC
Look for the table "NUMBER OF PROTECTED AUDIO_DATA BITS". You can open the
doc with any code editor, it isn't a binary word doc, just uses some 
pseudo-markdown syntax.

Reading bits for CRC isn't a common case, but I've encountered this before when 
trying
to make the AAC-HE decoder check the CRC, so maybe having a function in 
libavcodec or
libavutil for that would be a good idea.
___
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] fate: Update the test references for h264-skip-nokey and h264-skip-noparse after b48397e7b84864f2

[Michael Niedermayer]

On Mon, Aug 03, 2020 at 09:54:41AM +0300, Martin Storsjö wrote:
> ---
>  tests/ref/fate/h264-skip-nointra | 86 
>  tests/ref/fate/h264-skip-nokey   | 86 
>  2 files changed, 86 insertions(+), 86 deletions(-)

b48397e7b84864f2d4c70361a4c4bed93e826753 is wrong

if you update the checksums this may become more difficult
to cleanup. Not sure just a feeling ...

Please take a more carefull look at 
b48397e7b84864f2d4c70361a4c4bed93e826753

it does not disable crc checks for layer 1 & 2 it removes reading the CRC
field 
try decoding 
fate-suite//wtv/law-and-order-partial.wtv

it plays fine before but since b48397e7b84864f2d4c70361a4c4bed93e826753
its audio is broken

Thanks

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

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope


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] [FFmpeg-cvslog] mpegaudiodec_template: disable CRC checking for layers 1 and 2

[Lynne]

Aug 3, 2020, 08:58 by mar...@martin.st:

> On Sun, 2 Aug 2020, Lynne wrote:
>
>> ffmpeg | branch: master | Lynne  | Sun Aug  2 22:45:00 2020 
>> +0200| [b48397e7b84864f2d4c70361a4c4bed93e826753] | committer: Lynne
>>
>> mpegaudiodec_template: disable CRC checking for layers 1 and 2
>>
>> Layers 1 and 2 use lengths in bits which are not a multiple of 8,
>> and our CRC works on a per-byte basis.
>>
>>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=b48397e7b84864f2d4c70361a4c4bed93e826753
>>>
>> ---
>>
>> libavcodec/mpegaudiodec_template.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/mpegaudiodec_template.c 
>> b/libavcodec/mpegaudiodec_template.c
>> index 3d7e3ba4f2..f03d7bc275 100644
>> --- a/libavcodec/mpegaudiodec_template.c
>> +++ b/libavcodec/mpegaudiodec_template.c
>> @@ -1566,7 +1566,7 @@ static int mp_decode_frame(MPADecodeContext *s, 
>> OUT_INT **samples,
>>
>>  init_get_bits(&s->gb, buf + HEADER_SIZE, (buf_size - HEADER_SIZE) * 8);
>>
>> -if (s->error_protection) {
>> +if (s->error_protection && s->layer == 3) {
>>  uint16_t crc = get_bits(&s->gb, 16);
>>  if (s->err_recognition & AV_EF_CRCCHECK) {
>>  const int sec_len = s->lsf ? ((s->nb_channels == 1) ? 9  : 17) :
>>
>
> This change broke two fate tests; fate-h264-skip-nokey and 
> fate-h264-skip-nointra. The change does look sensible in itself though 
> (framecrcs that used to be all zeros now have nonzero, varying values).
>

Someone must have regen'd those tests after the patch was applied and thought
they were failing due to other changes.
___
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] fate: Update the test references for h264-skip-nokey and h264-skip-noparse after b48397e7b84864f2

[myp...@gmail.com]

On Mon, Aug 3, 2020 at 8:00 PM myp...@gmail.com  wrote:
>
> On Mon, Aug 3, 2020 at 2:54 PM Martin Storsjö  wrote:
> >
> > ---
> >  tests/ref/fate/h264-skip-nointra | 86 
> >  tests/ref/fate/h264-skip-nokey   | 86 
> >  2 files changed, 86 insertions(+), 86 deletions(-)
> >
> > diff --git a/tests/ref/fate/h264-skip-nointra 
> > b/tests/ref/fate/h264-skip-nointra
> > index 0259902927..723235ae6f 100644
> > --- a/tests/ref/fate/h264-skip-nointra
> > +++ b/tests/ref/fate/h264-skip-nointra
> > @@ -9,47 +9,47 @@
> >  #sample_rate 1: 48000
> >  #channel_layout 1: 3
> >  #channel_layout_name 1: stereo
> > -1,  0,  0, 1152, 4608, 0x
> > -1,   1152,   1152, 1152, 4608, 0x
> > -1,   2304,   2304, 1152, 4608, 0x
> > -1,   3456,   3456, 1152, 4608, 0x
> > -1,   4608,   4608, 1152, 4608, 0x
> > -1,   5760,   5760, 1152, 4608, 0x
> > -1,   6912,   6912, 1152, 4608, 0x
> > -1,   8064,   8064, 1152, 4608, 0x
> > -1,   9216,   9216, 1152, 4608, 0x
> > -1,  10368,  10368, 1152, 4608, 0x
> > -1,  11520,  11520, 1152, 4608, 0x
> > -1,  12672,  12672, 1152, 4608, 0x
> > -1,  13824,  13824, 1152, 4608, 0x
> > -1,  14976,  14976, 1152, 4608, 0x
> > -1,  16128,  16128, 1152, 4608, 0x
> > -1,  17280,  17280, 1152, 4608, 0x
> > -1,  18432,  18432, 1152, 4608, 0x
> > -1,  19584,  19584, 1152, 4608, 0x
> > -1,  20736,  20736, 1152, 4608, 0x
> > -1,  21888,  21888, 1152, 4608, 0x
> > -1,  23040,  23040, 1152, 4608, 0x
> > -1,  24192,  24192, 1152, 4608, 0x
> > -1,  25344,  25344, 1152, 4608, 0x
> > -1,  26496,  26496, 1152, 4608, 0x
> > -1,  27648,  27648, 1152, 4608, 0x
> > -1,  28800,  28800, 1152, 4608, 0x
> > -1,  29952,  29952, 1152, 4608, 0x
> > -1,  31104,  31104, 1152, 4608, 0x
> > -1,  32256,  32256, 1152, 4608, 0x
> > -1,  33408,  33408, 1152, 4608, 0x
> > -1,  34560,  34560, 1152, 4608, 0x
> > -1,  35712,  35712, 1152, 4608, 0x
> > -1,  36864,  36864, 1152, 4608, 0x
> > -1,  38016,  38016, 1152, 4608, 0x
> > -1,  39168,  39168, 1152, 4608, 0x
> > -1,  40320,  40320, 1152, 4608, 0x
> > -1,  41472,  41472, 1152, 4608, 0x
> > -1,  42624,  42624, 1152, 4608, 0x
> > -1,  43776,  43776, 1152, 4608, 0x
> > -1,  44928,  44928, 1152, 4608, 0x
> > -1,  46080,  46080, 1152, 4608, 0x
> > -1,  47232,  47232, 1152, 4608, 0x
> > -1,  48384,  48384, 1152, 4608, 0x9eca8b7c
> > +1,  0,  0, 1152, 4608, 0xa5e29a68
> > +1,   1152,   1152, 1152, 4608, 0x3738f584
> > +1,   2304,   2304, 1152, 4608, 0xb427c4ed
> > +1,   3456,   3456, 1152, 4608, 0xb9ded8e3
> > +1,   4608,   4608, 1152, 4608, 0x1951d6e4
> > +1,   5760,   5760, 1152, 4608, 0xeeabd125
> > +1,   6912,   6912, 1152, 4608, 0xd845ef87
> > +1,   8064,   8064, 1152, 4608, 0xac99d4e5
> > +1,   9216,   9216, 1152, 4608, 0x187cd4e5
> > +1,  10368,  10368, 1152, 4608, 0xc3c6bcf0
> > +1,  11520,  11520, 1152, 4608, 0xd4bedf1f
> > +1,  12672,  12672, 1152, 4608, 0xf185
> > +1,  13824,  13824, 1152, 4608, 0x487bd4e5
> > +1,  14976,  14976, 1152, 4608, 0x0cfacaea
> > +1,  16128,  16128, 1152, 4608, 0x9432d4e5
> > +1,  17280,  17280, 1152, 4608, 0x9e5dc12d
> > +1,  18432,  18432, 1152, 4608, 0x035ae58c
> > +1,  19584,  19584, 1152, 4608, 0x0db0bef0
> > +1,  20736,  20736, 1152, 4608, 0x081fd4e5
> > +1,  21888,  21888, 1152, 4608, 0x4ad4d6e3
> > +1,  23040,  23040, 1152, 4608, 0xa79dd325
> > +1,  24192,  24192, 1152, 4608, 0x6c77fb81
> > +1,  25344,  25344, 1152, 4608, 0xed25d2e5
> > +1,  26496,  26496, 1152, 4608, 0x9b94c2ee
> > +1,  27648,  27648, 1152, 4608, 0x9a3ad0e7
> > +1,  28800,  28800, 1152, 4608

Re: [FFmpeg-devel] [PATCH] fate: Update the test references for h264-skip-nokey and h264-skip-noparse after b48397e7b84864f2

[myp...@gmail.com]

On Mon, Aug 3, 2020 at 2:54 PM Martin Storsjö  wrote:
>
> ---
>  tests/ref/fate/h264-skip-nointra | 86 
>  tests/ref/fate/h264-skip-nokey   | 86 
>  2 files changed, 86 insertions(+), 86 deletions(-)
>
> diff --git a/tests/ref/fate/h264-skip-nointra 
> b/tests/ref/fate/h264-skip-nointra
> index 0259902927..723235ae6f 100644
> --- a/tests/ref/fate/h264-skip-nointra
> +++ b/tests/ref/fate/h264-skip-nointra
> @@ -9,47 +9,47 @@
>  #sample_rate 1: 48000
>  #channel_layout 1: 3
>  #channel_layout_name 1: stereo
> -1,  0,  0, 1152, 4608, 0x
> -1,   1152,   1152, 1152, 4608, 0x
> -1,   2304,   2304, 1152, 4608, 0x
> -1,   3456,   3456, 1152, 4608, 0x
> -1,   4608,   4608, 1152, 4608, 0x
> -1,   5760,   5760, 1152, 4608, 0x
> -1,   6912,   6912, 1152, 4608, 0x
> -1,   8064,   8064, 1152, 4608, 0x
> -1,   9216,   9216, 1152, 4608, 0x
> -1,  10368,  10368, 1152, 4608, 0x
> -1,  11520,  11520, 1152, 4608, 0x
> -1,  12672,  12672, 1152, 4608, 0x
> -1,  13824,  13824, 1152, 4608, 0x
> -1,  14976,  14976, 1152, 4608, 0x
> -1,  16128,  16128, 1152, 4608, 0x
> -1,  17280,  17280, 1152, 4608, 0x
> -1,  18432,  18432, 1152, 4608, 0x
> -1,  19584,  19584, 1152, 4608, 0x
> -1,  20736,  20736, 1152, 4608, 0x
> -1,  21888,  21888, 1152, 4608, 0x
> -1,  23040,  23040, 1152, 4608, 0x
> -1,  24192,  24192, 1152, 4608, 0x
> -1,  25344,  25344, 1152, 4608, 0x
> -1,  26496,  26496, 1152, 4608, 0x
> -1,  27648,  27648, 1152, 4608, 0x
> -1,  28800,  28800, 1152, 4608, 0x
> -1,  29952,  29952, 1152, 4608, 0x
> -1,  31104,  31104, 1152, 4608, 0x
> -1,  32256,  32256, 1152, 4608, 0x
> -1,  33408,  33408, 1152, 4608, 0x
> -1,  34560,  34560, 1152, 4608, 0x
> -1,  35712,  35712, 1152, 4608, 0x
> -1,  36864,  36864, 1152, 4608, 0x
> -1,  38016,  38016, 1152, 4608, 0x
> -1,  39168,  39168, 1152, 4608, 0x
> -1,  40320,  40320, 1152, 4608, 0x
> -1,  41472,  41472, 1152, 4608, 0x
> -1,  42624,  42624, 1152, 4608, 0x
> -1,  43776,  43776, 1152, 4608, 0x
> -1,  44928,  44928, 1152, 4608, 0x
> -1,  46080,  46080, 1152, 4608, 0x
> -1,  47232,  47232, 1152, 4608, 0x
> -1,  48384,  48384, 1152, 4608, 0x9eca8b7c
> +1,  0,  0, 1152, 4608, 0xa5e29a68
> +1,   1152,   1152, 1152, 4608, 0x3738f584
> +1,   2304,   2304, 1152, 4608, 0xb427c4ed
> +1,   3456,   3456, 1152, 4608, 0xb9ded8e3
> +1,   4608,   4608, 1152, 4608, 0x1951d6e4
> +1,   5760,   5760, 1152, 4608, 0xeeabd125
> +1,   6912,   6912, 1152, 4608, 0xd845ef87
> +1,   8064,   8064, 1152, 4608, 0xac99d4e5
> +1,   9216,   9216, 1152, 4608, 0x187cd4e5
> +1,  10368,  10368, 1152, 4608, 0xc3c6bcf0
> +1,  11520,  11520, 1152, 4608, 0xd4bedf1f
> +1,  12672,  12672, 1152, 4608, 0xf185
> +1,  13824,  13824, 1152, 4608, 0x487bd4e5
> +1,  14976,  14976, 1152, 4608, 0x0cfacaea
> +1,  16128,  16128, 1152, 4608, 0x9432d4e5
> +1,  17280,  17280, 1152, 4608, 0x9e5dc12d
> +1,  18432,  18432, 1152, 4608, 0x035ae58c
> +1,  19584,  19584, 1152, 4608, 0x0db0bef0
> +1,  20736,  20736, 1152, 4608, 0x081fd4e5
> +1,  21888,  21888, 1152, 4608, 0x4ad4d6e3
> +1,  23040,  23040, 1152, 4608, 0xa79dd325
> +1,  24192,  24192, 1152, 4608, 0x6c77fb81
> +1,  25344,  25344, 1152, 4608, 0xed25d2e5
> +1,  26496,  26496, 1152, 4608, 0x9b94c2ee
> +1,  27648,  27648, 1152, 4608, 0x9a3ad0e7
> +1,  28800,  28800, 1152, 4608, 0x0f6dcd27
> +1,  29952,  29952, 1152, 4608, 0xa72fe58c
> +1,  31104,  31104, 1152, 4608, 0x5950c2ee
> +1,  32256,  32256, 1152, 4608, 0x7006d6e4
> +1,  33408,  33408,   

Re: [FFmpeg-devel] [PATCH] [RFC] libavcodec/hevc_refs: Clear DPB of old frames

[Kieran Kunhya]

On Mon, 3 Aug 2020 at 09:18, Anton Khirnov  wrote:

> Quoting Kieran Kunhya (2020-07-16 21:32:34)
> > During glitching or looping streams, old frames remain in the DPB.
> > The decoder incorrectly thinks that the DPB contains the right number of
> > buffered frames to output and reordering breaks badly
> >
> > Any non-cosmetic comments welcome.
> >From 23f0272092f6528a268abe7e6a7fd8764553048f Mon Sep 17 00:00:00 2001
> >From: Kieran Kunhya 
> >Date: Thu, 16 Jul 2020 20:29:24 +0100
> >Subject: [PATCH] [RFC] libavcodec/hevc_refs: Clear DPB of old frames
> >
> >During glitching or looping streams, old frames remain in the DPB.
> >The decoder incorrectly thinks that the DPB contains the right number of
> buffered frames to output and reordering breaks badly
>
> Can you describe what happens in more details? It's not obvious what you
> "breaks badly" means. Even if some frame gets skipped, the old frames
> should still get output in the correct order, since their POCs are
> correct.
>

Here is my writeup:

The problem is that after the wrap around the DPB is not cleared correctly:

"DPB List: idx 0 poc -35588, idx 1 poc -7779, idx 2 poc -7772, idx 3 poc
-7768, idx 4 poc -7776, idx 5 poc -7780, idx 6 poc -7775, idx 7 poc -,
idx 8 poc -7781, idx 9 poc -7778, idx 10 poc -7774, idx 11 poc -7773, idx
12 poc -35592, idx 13 poc -35596, idx 14 poc -35599, idx 15 poc -35597, idx
16 poc -35601, idx 17 poc 0, idx 18 poc 0, idx 19 poc 0, idx 20 poc 0, idx
21 poc 0, idx 22 poc 0, idx 23 poc 0, idx 24 poc 0, idx 25 poc 0, idx 26
poc 0, idx 27 poc 0, idx 28 poc 0, idx 29 poc 0, idx 30 poc 0, idx 31 poc
0,"

The ~7000 series is the previous set of frames and after the wrap around
it's ~35000.
The logic here makes sure there is enough frames in the DPB before
outputting:
https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/hevc_refs.c#L203

But it gets confused because of the old frames in the DPB and thinks it's
ready to output. The logs show this as follows:

verbose: [102828010902015] [av] Output frame with POC -35588 dpb-idx 0.
verbose: [102828011180970] [av] Decoded frame with POC -35588.

This is wrong because the frame is decoded then immediately output so there
is no reordering that takes place.

During normal decode it looks more like this:

verbose: [102824827016306] [av] Output frame with POC -14866 dpb-idx 0.
verbose: [102824827130789] [av] Decoded frame with POC -14857.

(i.e there is reordering)


> >---
> > libavcodec/hevc_refs.c | 19 +++
> > 1 file changed, 19 insertions(+)
> >
> >diff --git a/libavcodec/hevc_refs.c b/libavcodec/hevc_refs.c
> >index 4f6d985..a8e0028 100644
> >--- a/libavcodec/hevc_refs.c
> >+++ b/libavcodec/hevc_refs.c
> >@@ -277,6 +277,10 @@ static int init_slice_rpl(HEVCContext *s)
> > int ctb_addr_ts  =
> s->ps.pps->ctb_addr_rs_to_ts[s->sh.slice_segment_addr];
> > int i;
> >
> >+if(frame && !frame->rpl_buf) {
>
> How can this happen? From looking at the code, I don't see how either of
> those conditions can possibly be true.
>

From memory I had segfaults if that check was not there.

Kieran
___
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] [v5] dnn_backend_native_layer_mathunary: add ceil support

[Mingyu Yin]

It can be tested with the model generated with below python script:

import tensorflow as tf
import os
import numpy as np
import imageio
from tensorflow.python.framework import graph_util
name = 'ceil'

pb_file_path = os.getcwd()
if not os.path.exists(pb_file_path+'/{}_savemodel/'.format(name)):
os.mkdir(pb_file_path+'/{}_savemodel/'.format(name))

with tf.Session(graph=tf.Graph()) as sess:
in_img = imageio.imread('detection.jpg')
in_img = in_img.astype(np.float32)
in_data = in_img[np.newaxis, :]
input_x = tf.placeholder(tf.float32, shape=[1, None, None, 3], 
name='dnn_in')
y_ = tf.math.ceil(input_x*255)/255
y = tf.identity(y_, name='dnn_out')
sess.run(tf.global_variables_initializer())
constant_graph = graph_util.convert_variables_to_constants(sess, 
sess.graph_def, ['dnn_out'])

with tf.gfile.FastGFile(pb_file_path+'/{}_savemodel/model.pb'.format(name), 
mode='wb') as f:
f.write(constant_graph.SerializeToString())

print("model.pb generated, please in ffmpeg path use\n \n \
python tools/python/convert.py ceil_savemodel/model.pb 
--outdir=ceil_savemodel/ \n \nto generate model.model\n")

output = sess.run(y, feed_dict={ input_x: in_data})
imageio.imsave("out.jpg", np.squeeze(output))

print("To verify, please ffmpeg path use\n \n \
./ffmpeg -i detection.jpg -vf 
format=rgb24,dnn_processing=model=ceil_savemodel/model.pb:input=dnn_in:output=dnn_out:dnn_backend=tensorflow
 -f framemd5 ceil_savemodel/tensorflow_out.md5\n  \
or\n \
./ffmpeg -i detection.jpg -vf 
format=rgb24,dnn_processing=model=ceil_savemodel/model.pb:input=dnn_in:output=dnn_out:dnn_backend=tensorflow
 ceil_savemodel/out_tensorflow.jpg\n \nto generate output result of tensorflow 
model\n")

print("To verify, please ffmpeg path use\n \n \
./ffmpeg -i detection.jpg -vf 
format=rgb24,dnn_processing=model=ceil_savemodel/model.model:input=dnn_in:output=dnn_out:dnn_backend=native
 -f framemd5 ceil_savemodel/native_out.md5\n  \
or \n \
./ffmpeg -i detection.jpg -vf 
format=rgb24,dnn_processing=model=ceil_savemodel/model.model:input=dnn_in:output=dnn_out:dnn_backend=native
 ceil_savemodel/out_native.jpg\n \nto generate output result of native model\n")

Signed-off-by: Mingyu Yin 
---
 libavfilter/dnn/dnn_backend_native_layer_mathunary.c | 4 
 libavfilter/dnn/dnn_backend_native_layer_mathunary.h | 1 +
 tests/dnn/dnn-layer-mathunary-test.c | 4 
 tools/python/convert_from_tensorflow.py  | 4 +++-
 tools/python/convert_header.py   | 2 +-
 5 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/libavfilter/dnn/dnn_backend_native_layer_mathunary.c 
b/libavfilter/dnn/dnn_backend_native_layer_mathunary.c
index c5f0f7adec..a62f6ba6f0 100644
--- a/libavfilter/dnn/dnn_backend_native_layer_mathunary.c
+++ b/libavfilter/dnn/dnn_backend_native_layer_mathunary.c
@@ -130,6 +130,10 @@ int dnn_execute_layer_math_unary(DnnOperand *operands, 
const int32_t *input_oper
 for (int i = 0; i < dims_count; ++i)
 dst[i] = atanh(src[i]);
 return 0;
+case DMUO_CEIL:
+for (int i = 0; i < dims_count; ++i)
+dst[i] = ceil(src[i]);
+return 0;
 default:
 return -1;
 }
diff --git a/libavfilter/dnn/dnn_backend_native_layer_mathunary.h 
b/libavfilter/dnn/dnn_backend_native_layer_mathunary.h
index 8076356ba4..82b2d7f4ab 100644
--- a/libavfilter/dnn/dnn_backend_native_layer_mathunary.h
+++ b/libavfilter/dnn/dnn_backend_native_layer_mathunary.h
@@ -43,6 +43,7 @@ typedef enum {
 DMUO_ASINH = 10,
 DMUO_ACOSH = 11,
 DMUO_ATANH = 12,
+DMUO_CEIL = 13,
 DMUO_COUNT
 } DNNMathUnaryOperation;
 
diff --git a/tests/dnn/dnn-layer-mathunary-test.c 
b/tests/dnn/dnn-layer-mathunary-test.c
index 5afc5c157e..7da3a206ed 100644
--- a/tests/dnn/dnn-layer-mathunary-test.c
+++ b/tests/dnn/dnn-layer-mathunary-test.c
@@ -56,6 +56,8 @@ static float get_expected(float f, DNNMathUnaryOperation op)
 return acosh(f);
 case DMUO_ATANH:
 return atanh(f);
+case DMUO_CEIL:
+return ceil(f);
 default:
 av_assert0(!"not supported yet");
 return 0.f;
@@ -128,5 +130,7 @@ int main(int agrc, char **argv)
 return 1;
 if (test(DMUO_ATANH))
 return 1;
+if (test(DMUO_CEIL))
+return 1;
 return 0;
 }
diff --git a/tools/python/convert_from_tensorflow.py 
b/tools/python/convert_from_tensorflow.py
index 85db7bf710..64b7551314 100644
--- a/tools/python/convert_from_tensorflow.py
+++ b/tools/python/convert_from_tensorflow.py
@@ -72,7 +72,9 @@ class TFConverter:
 self.conv2d_scopename_inputname_dict = {}
 self.op2code = {'Conv2D':1, 'DepthToSpace':2, 'MirrorPad':3, 
'Maximum':4, 'MathBinary':5, 'MathUnary':6}
 self.mathbin2code = {'Sub':0, 'Add':1, 'Mul':2, 'RealDiv':3, 
'Minimum':4}
-self.mathun2code  = {'Abs':0, 'Sin':1, 'Cos':2, 'Tan':3, 'Asin':4, 
'Acos':5, 'Atan':6, 'S

Re: [FFmpeg-devel] [PATCH] fate/aac: add missing bitexact flag to some encoder tests

[Paul B Mahol]

On 8/3/20, James Almer  wrote:
> On 8/2/2020 8:43 PM, James Almer wrote:
>> Will prevet FATE from breaking once LIBAVCODEC_VERSION_MINOR is bumped to
>> 100.
>>
>> Reported-by: zane
>> Signed-off-by: James Almer 
>> ---
>> If anyone wants to check why adding LIBAVCODEC_IDENT to the bitstream (or
>> rather,
>> its length increasing) in either
>> https://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavcodec/aacenc.c;h=e65b76cd74ab3cf86f23de0528cce115e91350f3;hb=HEAD#l60
>> or
>> https://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavcodec/aacenc.c;h=e65b76cd74ab3cf86f23de0528cce115e91350f3;hb=HEAD#l689
>> results in these tests failing when comparing the decoded output with the
>> reference file, that would be great, because i don't think it should
>> happen.
>>
>>  libavcodec/version.h |  2 +-
>>  tests/fate/aac.mak   | 12 ++--
>>  2 files changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/libavcodec/version.h b/libavcodec/version.h
>> index f66919617a..a3f9f828ee 100644
>> --- a/libavcodec/version.h
>> +++ b/libavcodec/version.h
>> @@ -28,7 +28,7 @@
>>  #include "libavutil/version.h"
>>
>>  #define LIBAVCODEC_VERSION_MAJOR  58
>> -#define LIBAVCODEC_VERSION_MINOR  99
>> +#define LIBAVCODEC_VERSION_MINOR 100
>
> Err, of course this change is not meant to be in this patch. Forgot to
> remove it while testing. Did so locally just now.
>
>>  #define LIBAVCODEC_VERSION_MICRO 100
>>
>>  #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR,
>> \
>> diff --git a/tests/fate/aac.mak b/tests/fate/aac.mak
>> index bd283225c1..4b38d3648c 100644
>> --- a/tests/fate/aac.mak
>> +++ b/tests/fate/aac.mak
>> @@ -150,7 +150,7 @@ FATE_AAC += $(FATE_AAC_CT:%=fate-aac-ct-%)
>>
>>  FATE_AAC_ENCODE += fate-aac-aref-encode
>>  fate-aac-aref-encode: ./tests/data/asynth-44100-2.wav
>> -fate-aac-aref-encode: CMD = enc_dec_pcm adts wav s16le $(REF) -c:a aac
>> -aac_is 0 -aac_pns 0 -aac_ms 0 -aac_tns 0 -b:a 512k
>> +fate-aac-aref-encode: CMD = enc_dec_pcm adts wav s16le $(REF) -c:a aac
>> -aac_is 0 -aac_pns 0 -aac_ms 0 -aac_tns 0 -b:a 512k -fflags +bitexact
>> -flags +bitexact
>>  fate-aac-aref-encode: CMP = stddev
>>  fate-aac-aref-encode: REF = ./tests/data/asynth-44100-2.wav
>>  fate-aac-aref-encode: CMP_SHIFT = -4096
>> @@ -159,7 +159,7 @@ fate-aac-aref-encode: SIZE_TOLERANCE = 2464
>>  fate-aac-aref-encode: FUZZ = 89
>>
>>  FATE_AAC_ENCODE += fate-aac-ln-encode
>> -fate-aac-ln-encode: CMD = enc_dec_pcm adts wav s16le
>> $(TARGET_SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav -c:a aac
>> -aac_is 0 -aac_pns 0 -aac_ms 0 -aac_tns 0 -b:a 512k
>> +fate-aac-ln-encode: CMD = enc_dec_pcm adts wav s16le
>> $(TARGET_SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav -c:a aac
>> -aac_is 0 -aac_pns 0 -aac_ms 0 -aac_tns 0 -b:a 512k -fflags +bitexact
>> -flags +bitexact
>>  fate-aac-ln-encode: CMP = stddev
>>  fate-aac-ln-encode: REF =
>> $(SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav
>>  fate-aac-ln-encode: CMP_SHIFT = -4096
>> @@ -168,7 +168,7 @@ fate-aac-ln-encode: SIZE_TOLERANCE = 3560
>>  fate-aac-ln-encode: FUZZ = 30
>>
>>  FATE_AAC_ENCODE += fate-aac-ln-encode-128k
>> -fate-aac-ln-encode-128k: CMD = enc_dec_pcm adts wav s16le
>> $(TARGET_SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav -c:a aac
>> -aac_is 0 -aac_pns 0 -aac_ms 0 -aac_tns 0 -b:a 128k -cutoff 22050
>> +fate-aac-ln-encode-128k: CMD = enc_dec_pcm adts wav s16le
>> $(TARGET_SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav -c:a aac
>> -aac_is 0 -aac_pns 0 -aac_ms 0 -aac_tns 0 -b:a 128k -cutoff 22050 -fflags
>> +bitexact -flags +bitexact
>>  fate-aac-ln-encode-128k: CMP = stddev
>>  fate-aac-ln-encode-128k: REF =
>> $(SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav
>>  fate-aac-ln-encode-128k: CMP_SHIFT = -4096
>> @@ -195,7 +195,7 @@ fate-aac-tns-encode: FUZZ = 7
>>  fate-aac-tns-encode: SIZE_TOLERANCE = 3560
>>
>>  FATE_AAC_ENCODE += fate-aac-is-encode
>> -fate-aac-is-encode: CMD = enc_dec_pcm adts wav s16le
>> $(TARGET_SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav -c:a aac
>> -aac_pns 0 -aac_is 1 -aac_ms 0 -b:a 128k -aac_tns 0 -cutoff 22050
>> +fate-aac-is-encode: CMD = enc_dec_pcm adts wav s16le
>> $(TARGET_SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav -c:a aac
>> -aac_pns 0 -aac_is 1 -aac_ms 0 -b:a 128k -aac_tns 0 -cutoff 22050 -fflags
>> +bitexact -flags +bitexact
>>  fate-aac-is-encode: CMP = stddev
>>  fate-aac-is-encode: REF =
>> $(SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav
>>  fate-aac-is-encode: CMP_SHIFT = -4096
>> @@ -204,7 +204,7 @@ fate-aac-is-encode: SIZE_TOLERANCE = 3560
>>  fate-aac-is-encode: FUZZ = 10
>>
>>  FATE_AAC_ENCODE += fate-aac-ms-encode
>> -fate-aac-ms-encode: CMD = enc_dec_pcm adts wav s16le
>> $(TARGET_SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav -c:a aac
>> -aac_pns 0 -aac_is 0 -aac_ms 1 -aac_tns 0 -b:a 128k -cutoff 22050
>> +fate-aac-ms-encode: CMD = enc_dec_pcm adts wav s16le
>> $(TARGET_SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav -c:a aa

Re: [FFmpeg-devel] [PATCH 1/5] avutil/channel_layout: add 22.2 layout

[Jan Ekström]

On Mon, Aug 3, 2020 at 10:34 AM Carl Eugen Hoyos  wrote:
>
>
>
> > Am 02.08.2020 um 23:16 schrieb Jan Ekström :
> >
> >> On Sat, Aug 1, 2020 at 4:29 PM Carl Eugen Hoyos  wrote:
> >>
> >>> Am Sa., 1. Aug. 2020 um 14:47 Uhr schrieb Jan Ekström :
> >>>
>  On Sat, Aug 1, 2020 at 3:08 PM Carl Eugen Hoyos  
>  wrote:
> 
> > Am Sa., 1. Aug. 2020 um 14:06 Uhr schrieb Jan Ekström 
> > :
> >
> > Additionally, the spec only mentions stereo/5.1 for direct mapping, so
> > I've kept to 5.1 since we have the capability for that one.
> 
>  And you still believe it would be a disadvantage if the decoder outputs
>  this 5.1 mapping by default if nothing else was requested?
> >>>
> >>> Yes. API users so far have received the audio frames according to
> >>> encoded layout by default, so doing something else breaks that rule of
> >>> least surprises.
> >>>
>  Is there an option to request the 5.1 mapping?
> >>>
> >>> As noted in the 0/0 cover letter, you can do it just fine with
> >>> `-channel_layout "5.1"` with ffmpeg.c. I have verified that it returns
> >>> the 5.1 channels as-is.
> >>
> >> But this does not make the decoder output 5.1 or does it?
> >>
> >> While I should probably only care about ffmpeg and ignore
> >> the library users I still wonder who can live with your approach...
> >>
> >
> > As this can be implied to be a comment regarding me not caring about
> > the API users,
>
> It was a comment about me not understanding how the average of hundreds of 
> library users will be able to use this - important - new feature.
>
> > and generally being wording that can be interpreted as
> > disagreeing on the whole approach of the patch set, please respond
> > whether this is a blocker or not.
>
> Definitely not and I am sorry if my wording was so bad.
>
> Carl Eugen

Thank you for confirming that this is not a blocker.

Jan
___
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] doc/developer: origin of tables should be documented.

[Nicolas George]

Nicolas George (12020-07-31):
> Something like this would be acceptable:
> 
> /* Reverse-engineered by encoding various files with the reference
>encoder. */
> 
> Reluctance to give this little information is baffling to me.

Can other developers give their input on this?

Is it acceptable or too much to require at least short comments like
these ones before tables of numbers in the source code?

/* Copied from https://oeis.org/A55 */

/* Computed with =IF(MOD(A1;2)=0;A1/2;3*A1+1) */

/* Adjusted to match decoding of samples/$dir/*.xxx with screenshots
   from the game. */

They are IMHO necessary to let other people learn from the source code,
which is the difference between Libre Software and non-Libre Software.
They may probably be useful for the author themself if they come back
much later.

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".

[FFmpeg-devel] [PATCH] avcodec/cfhd: use LUT for 9 and 18 codebook decompanding

[Paul B Mahol]

Hi,

patch attached.


0001-avcodec-cfhd-use-LUT-for-9-and-18-codebook-decompand.patch
Description: Binary data
___
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] lavu/buffer: add a convenience function for replacing buffers

[Anton Khirnov]

A common pattern e.g. in libavcodec is replacing/updating buffer
references: unref old one, ref new one. This function allows simplifying
such code an avoiding unnecessary refs+unrefs if the references are
already equivalent.
---
Updated to not touch dst on failure.

Still not using buffer_replace(), since it does not seem to make the
code simpler.
---
 doc/APIchanges  |  3 +++
 libavutil/buffer.c  | 26 ++
 libavutil/buffer.h  | 16 
 libavutil/version.h |  2 +-
 4 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index 72a4833fbc..5a3f0165a8 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,9 @@ libavutil: 2017-10-21
 
 API changes, most recent first:
 
+2020-xx-xx - xx - lavu 56.51.100 - buffer.h
+  Add a av_buffer_replace() convenience function.
+
 2020-07-xx - xx - lavu 56.57.100 - cpu.h
   Add AV_CPU_FLAG_MMI and AV_CPU_FLAG_MSA.
 
diff --git a/libavutil/buffer.c b/libavutil/buffer.c
index 38a554208a..39539f99e9 100644
--- a/libavutil/buffer.c
+++ b/libavutil/buffer.c
@@ -215,6 +215,32 @@ int av_buffer_realloc(AVBufferRef **pbuf, int size)
 return 0;
 }
 
+int av_buffer_replace(AVBufferRef **pdst, AVBufferRef *src)
+{
+AVBufferRef *dst = *pdst;
+AVBufferRef *tmp;
+
+if (!src) {
+av_buffer_unref(pdst);
+return 0;
+}
+
+if (dst && dst->buffer == src->buffer) {
+/* make sure the data pointers match */
+dst->data = src->data;
+dst->size = src->size;
+return 0;
+}
+
+tmp = av_buffer_ref(src);
+if (!tmp)
+return AVERROR(ENOMEM);
+
+av_buffer_unref(pdst);
+*pdst = tmp;
+return 0;
+}
+
 AVBufferPool *av_buffer_pool_init2(int size, void *opaque,
AVBufferRef* (*alloc)(void *opaque, int 
size),
void (*pool_free)(void *opaque))
diff --git a/libavutil/buffer.h b/libavutil/buffer.h
index c0f3f6cc9a..fd4e381efa 100644
--- a/libavutil/buffer.h
+++ b/libavutil/buffer.h
@@ -197,6 +197,22 @@ int av_buffer_make_writable(AVBufferRef **buf);
  */
 int av_buffer_realloc(AVBufferRef **buf, int size);
 
+/**
+ * Ensure dst refers to the same data as src.
+ *
+ * When *dst is already equivalent to src, do nothing. Otherwise unreference 
dst
+ * and replace it with a new reference to src.
+ *
+ * @param dst Pointer to either a valid buffer reference or NULL. On success,
+ *this will point to a buffer reference equivalent to src. On
+ *failure, dst will be left untouched.
+ * @param src A buffer reference to replace dst with. May be NULL, then this
+ *function is equivalent to av_buffer_unref(dst).
+ * @return 0 on success
+ * AVERROR(ENOMEM) on memory allocation failure.
+ */
+int av_buffer_replace(AVBufferRef **dst, AVBufferRef *src);
+
 /**
  * @}
  */
diff --git a/libavutil/version.h b/libavutil/version.h
index 975fb87f31..3e7e1f410b 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@
  */
 
 #define LIBAVUTIL_VERSION_MAJOR  56
-#define LIBAVUTIL_VERSION_MINOR  57
+#define LIBAVUTIL_VERSION_MINOR  58
 #define LIBAVUTIL_VERSION_MICRO 100
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
-- 
2.27.0

___
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 1/5] avutil/channel_layout: add 22.2 layout

[Hendrik Leppkes]

On Mon, Aug 3, 2020 at 9:34 AM Carl Eugen Hoyos  wrote:
>
>
>
> > Am 02.08.2020 um 23:16 schrieb Jan Ekström :
> >
> >> On Sat, Aug 1, 2020 at 4:29 PM Carl Eugen Hoyos  wrote:
> >>
> >>> Am Sa., 1. Aug. 2020 um 14:47 Uhr schrieb Jan Ekström :
> >>>
>  On Sat, Aug 1, 2020 at 3:08 PM Carl Eugen Hoyos  
>  wrote:
> 
> > Am Sa., 1. Aug. 2020 um 14:06 Uhr schrieb Jan Ekström 
> > :
> >
> > Additionally, the spec only mentions stereo/5.1 for direct mapping, so
> > I've kept to 5.1 since we have the capability for that one.
> 
>  And you still believe it would be a disadvantage if the decoder outputs
>  this 5.1 mapping by default if nothing else was requested?
> >>>
> >>> Yes. API users so far have received the audio frames according to
> >>> encoded layout by default, so doing something else breaks that rule of
> >>> least surprises.
> >>>
>  Is there an option to request the 5.1 mapping?
> >>>
> >>> As noted in the 0/0 cover letter, you can do it just fine with
> >>> `-channel_layout "5.1"` with ffmpeg.c. I have verified that it returns
> >>> the 5.1 channels as-is.
> >>
> >> But this does not make the decoder output 5.1 or does it?
> >>
> >> While I should probably only care about ffmpeg and ignore
> >> the library users I still wonder who can live with your approach...
> >>
> >
> > As this can be implied to be a comment regarding me not caring about
> > the API users,
>
> It was a comment about me not understanding how the average of hundreds of 
> library users will be able to use this - important - new feature.
>

They can use it like they use any other uncommon channel layout, by
using lavfi or swresample directly to remix the audio - the same
process that ffmpeg.c uses internally.

- 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] [FFmpeg-cvslog] mpegaudiodec_template: disable CRC checking for layers 1 and 2

[Martin Storsjö]

On Mon, 3 Aug 2020, Lynne wrote:


Aug 3, 2020, 08:58 by mar...@martin.st:

This change broke two fate tests; fate-h264-skip-nokey and 
fate-h264-skip-nointra. The change does look sensible in itself though 
(framecrcs that used to be all zeros now have nonzero, varying values).




Someone must have regen'd those tests after the patch was applied and thought
they were failing due to other changes.


I'm not quite sure what you refer to - there's no other commits after 
yours in git, and all fate instances are turning yellow.


I posted a patch to regenerate those test references, but it's not applied 
yet, pending an ack from somebody.


// Martin

___
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] [RFC] libavcodec/hevc_refs: Clear DPB of old frames

[Anton Khirnov]

Quoting Kieran Kunhya (2020-07-16 21:32:34)
> During glitching or looping streams, old frames remain in the DPB.
> The decoder incorrectly thinks that the DPB contains the right number of
> buffered frames to output and reordering breaks badly
> 
> Any non-cosmetic comments welcome.
>From 23f0272092f6528a268abe7e6a7fd8764553048f Mon Sep 17 00:00:00 2001
>From: Kieran Kunhya 
>Date: Thu, 16 Jul 2020 20:29:24 +0100
>Subject: [PATCH] [RFC] libavcodec/hevc_refs: Clear DPB of old frames
>
>During glitching or looping streams, old frames remain in the DPB.
>The decoder incorrectly thinks that the DPB contains the right number of 
>buffered frames to output and reordering breaks badly

Can you describe what happens in more details? It's not obvious what you
"breaks badly" means. Even if some frame gets skipped, the old frames
should still get output in the correct order, since their POCs are
correct.

>---
> libavcodec/hevc_refs.c | 19 +++
> 1 file changed, 19 insertions(+)
>
>diff --git a/libavcodec/hevc_refs.c b/libavcodec/hevc_refs.c
>index 4f6d985..a8e0028 100644
>--- a/libavcodec/hevc_refs.c
>+++ b/libavcodec/hevc_refs.c
>@@ -277,6 +277,10 @@ static int init_slice_rpl(HEVCContext *s)
> int ctb_addr_ts  = s->ps.pps->ctb_addr_rs_to_ts[s->sh.slice_segment_addr];
> int i;
> 
>+if(frame && !frame->rpl_buf) {

How can this happen? From looking at the code, I don't see how either of
those conditions can possibly be true.

-- 
Anton Khirnov
___
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 1/5] avutil/channel_layout: add 22.2 layout

[Carl Eugen Hoyos]

> Am 02.08.2020 um 23:16 schrieb Jan Ekström :
> 
>> On Sat, Aug 1, 2020 at 4:29 PM Carl Eugen Hoyos  wrote:
>> 
>>> Am Sa., 1. Aug. 2020 um 14:47 Uhr schrieb Jan Ekström :
>>> 
 On Sat, Aug 1, 2020 at 3:08 PM Carl Eugen Hoyos  wrote:
 
> Am Sa., 1. Aug. 2020 um 14:06 Uhr schrieb Jan Ekström :
> 
> Additionally, the spec only mentions stereo/5.1 for direct mapping, so
> I've kept to 5.1 since we have the capability for that one.
 
 And you still believe it would be a disadvantage if the decoder outputs
 this 5.1 mapping by default if nothing else was requested?
>>> 
>>> Yes. API users so far have received the audio frames according to
>>> encoded layout by default, so doing something else breaks that rule of
>>> least surprises.
>>> 
 Is there an option to request the 5.1 mapping?
>>> 
>>> As noted in the 0/0 cover letter, you can do it just fine with
>>> `-channel_layout "5.1"` with ffmpeg.c. I have verified that it returns
>>> the 5.1 channels as-is.
>> 
>> But this does not make the decoder output 5.1 or does it?
>> 
>> While I should probably only care about ffmpeg and ignore
>> the library users I still wonder who can live with your approach...
>> 
> 
> As this can be implied to be a comment regarding me not caring about
> the API users,

It was a comment about me not understanding how the average of hundreds of 
library users will be able to use this - important - new feature.

> and generally being wording that can be interpreted as
> disagreeing on the whole approach of the patch set, please respond
> whether this is a blocker or not.

Definitely not and I am sorry if my wording was so bad.

Carl Eugen
___
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] Inconsistent handling of initial_padding and PTS adjustment

[Anton Khirnov]

Quoting Kieran Kunhya (2020-07-27 20:15:17)
> On Mon, 27 Jul 2020 at 11:09, Anton Khirnov  wrote:
> 
> > Quoting Kieran Kunhya (2020-07-26 01:51:22)
> > > Hi,
> > >
> > > I notice that some encoders adjust the PTS with initial_padding and some
> > > don't.
> > > Is this intentional and should we decide that all encoders should do
> > this?
> >
> > Which ones don't?
> >
> 
> A few here:
> https://github.com/FFmpeg/FFmpeg/search?p=1&q=initial_padding&unscoped_q=initial_padding

Sounds a lot like "find them yourself". Presumably you already did that,
so might as well list at least an example.

> > I don't think this is a matter of opinion really - if encoder adds
> > padding and doesn't adjust the timestamps then the output timestamps are
> > just plain wrong.
> >
> 
> Well some containers have a flag for it. Right now if you encoded with
> libopus into mkv or ts you get the PTS offset as well as the syntax element
> written to the bitstream.

Then I'd say if a container has a special field for padding then it
should also adjust timestamps.

-- 
Anton Khirnov
___
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".