Re: [FFmpeg-devel] [PATCH V1 2/3] doc/ffmpeg: Document dts_error_threshold option

2019-07-24 Thread myp...@gmail.com
On Wed, Jul 24, 2019 at 5:08 PM Gyan  wrote:
>
>
>
> On 24-07-2019 01:38 PM, Michael Niedermayer wrote:
> > On Sun, Jul 21, 2019 at 10:31:36PM +0800, Jun Zhao wrote:
> >> From: Jun Zhao 
> >>
> >> Document dts_error_threshold option.
> >>
> >> Signed-off-by: Jun Zhao 
> >> ---
> >>   doc/ffmpeg.texi |2 ++
> >>   1 files changed, 2 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> >> index cd35eb4..2152e62 100644
> >> --- a/doc/ffmpeg.texi
> >> +++ b/doc/ffmpeg.texi
> >> @@ -1515,6 +1515,8 @@ Enable bitexact mode for (de)muxer and (de/en)coder
> >>   Finish encoding when the shortest input stream ends.
> >>   @item -dts_delta_threshold
> >>   Timestamp discontinuity delta threshold.
> >> +@item -dts_error_threshold
> >> +Timestamp error delta threshold.
> > ok, though a bit terse
> >
> Yes, mention what this option actually does. Mention units and if
> applicable, range.
>
Will add other part as the comments. Tks
___
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] avcodec: add a WavPack DSD decoder

2019-07-24 Thread David Bryant
On 7/21/19 5:57 PM, Lynne wrote:
> Jul 22, 2019, 12:03 AM by da...@wavpack.com:
>
>> Hi,
>>
>> As I promised late last year, here is a patch to add a WavPack DSD decoder.
>>
>> Thanks!
>>
>> -David Bryant
>>
>> +    unsigned char probabilities [MAX_HISTORY_BINS] [256];
>> +    unsigned char *value_lookup [MAX_HISTORY_BINS];
> Use uint8_t throughout the patch.
> Also don't add spaces between array declarations or lookups.
Done. New patch attached for all changes (hope that's the right way to do it).
>
>
>> +static void init_ptable (int *table, int rate_i, int rate_s)
>> +{
>> +    int value = 0x808000, rate = rate_i << 8, c, i;
>> +
>> +    for (c = (rate + 128) >> 8; c--;)
>> +    value += (DOWN - value) >> DECAY;
>> +
>> +    for (i = 0; i < PTABLE_BINS/2; ++i) {
> What's up with the random increment position in loops? It changes to before 
> and after the variable throughout. Make it consistent and after the variable.
> Also we support declarative for (int loops. Can save lines.

Fixed random increment position and put in declarative int loops (even when 
they didn't save a line).


>
>
>> +    DSDfilters filters [2], *sp = filters;
> Same, spaces after variables for arrays, all throughout the file.

Done. Also fixed spaces after function names.


>
>
>> +    if (code > max_probability) {
>> +    int zcount = code - max_probability;
>> +
>> +    while (outptr < outend && zcount--)
>> +    *outptr++ = 0;
>> +    }
>> +    else if (code)
>> +    *outptr++ = code;
>> +    else
>> +    break;
> We don't put else on a new line, and prefer to have each branch wrapped in 
> bracket unless all branches are single lines.

Fixed.


>
>
>> +    for (p0 = 0; p0 < history_bins; ++p0) {
>> +    int32_t sum_values;
>> +    unsigned char *vp;
>> +
>> +    for (sum_values = i = 0; i < 256; ++i)
>> +    s->summed_probabilities [p0] [i] = sum_values += 
>> s->probabilities [p0] [i];
> sum_values is uninitialized. Does you compiler not warn about this?

This was pointed out already as actually initialized, but I made it clearer 
(obviously it wasn't).


>
>
>> +    if (sum_values) {
>> +    total_summed_probabilities += sum_values;
>> +    vp = s->value_lookup [p0] = av_malloc (sum_values);
> I don't like the per-frame alloc. The maximum sum_values can be is 255*255 = 
> UINT16_MAX.
>  60k of memory isn't much at all, just define value_lookup[255*255] in the 
> context and you'll probably plug a few out of bounds accesses too.

It's actually up to 32 allocs per frame because there's one for each history 
bin (value_lookup is an array of pointers, not
uint8s), and I didn't like it either because I had to worry about de-allocing 
on error. Refactored to use a single array in the
context as a pool. Thanks for the suggestion!


>
>
>> +mult = high / s->summed_probabilities [p0] [255];
> s->summed_probabilities [p0] [255]; can be zero, you already check if its 
> zero when allocating currently. You should probably check for divide by zero 
> unless you're very sure it can't happen.

I'm very sure. The checks are within a few lines above each of the three 
divides.


>
>
>> +    crc += (crc << 1) + code;
> Don't NIH CRCs, we have av_crc in lavu. See below how to use it.

It's not a standard crc, but more of a recirculating checksum, so the NIH code 
is required.


>
>
>> +static int wv_unpack_dsd_copy(WavpackFrameContext *s, void *dst_l, void 
>> *dst_r)
>> +{
>> +    uint8_t *dsd_l  = dst_l;
>> +    uint8_t *dsd_r  = dst_r;
> You're shadowing arguments. Your compiler doesn't warn on this either?
> You're calling the function with uint8_ts anyway, just change the type.

They're not shadowed (dsd vs. dst) which is why my compiler didn't complain, 
but I took your suggestion of just changing the types.


>
>
>> +    while (total_samples--) {
>> +    crc += (crc << 1) + (*dsd_l = bytestream2_get_byte(>gb));
>> +    dsd_l += 4;
>> +
>> +    if (dst_r) {
>> +    crc += (crc << 1) + (*dsd_r = bytestream2_get_byte(>gb));
>> +    dsd_r += 4;
>> +    }
>> +    }
> av_crc(av_crc_get_table(AV_CRC_32_IEEE/LE), UINT32_MAX, dsd_start_r/l, 
> dsd_r/l - dsd_start_r/l) should work and be faster.

see above


>
>
>> +    s->fdec_num = 0;
> Private codec context is always zeroed already.

removed


>
>
>> +    int chan = 0, chmask = 0, sample_rate = 0, rate_x = 1, dsd_mode = 0;
>> +    chmask = avctx->channel_layout;
>>   uint32_t chmask, flags;
> frame->channel_layout is uint64_t.

good to know...fixed


>
>
>> +    samples_l = frame->extended_data[wc->ch_offset];
>> +    if (s->stereo)
>> +    samples_r = frame->extended_data[wc->ch_offset + 1];
>> +
>> +    wc->ch_offset += 1 + s->stereo;
> Have you checked non-stereo decodes fine and the channels are correctly 
> ordered?

Yes.


>
>
>> +    if (id & 

[FFmpeg-devel] [PATCH] avformat/hlsenc: Fix overflow of int for durations compute

2019-07-24 Thread Steven Liu
Fix ticket: 8037

Signed-off-by: Steven Liu 
---
 libavformat/hlsenc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index 2ade6723f9..51310fb528 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -2301,8 +2301,8 @@ static int hls_write_packet(AVFormatContext *s, AVPacket 
*pkt)
 
 if (vs->sequence - vs->nb_entries > hls->start_sequence && hls->init_time 
> 0) {
 /* reset end_pts, hls->recording_time at end of the init hls list */
-int init_list_dur = hls->init_time * vs->nb_entries * AV_TIME_BASE;
-int after_init_list_dur = (vs->sequence - hls->start_sequence - 
vs->nb_entries ) * (hls->time * AV_TIME_BASE);
+int64_t init_list_dur = hls->init_time * vs->nb_entries * AV_TIME_BASE;
+int64_t after_init_list_dur = (vs->sequence - hls->start_sequence - 
vs->nb_entries ) * (hls->time * AV_TIME_BASE);
 hls->recording_time = hls->time * AV_TIME_BASE;
 end_pts = init_list_dur + after_init_list_dur ;
 }
-- 
2.17.2 (Apple Git-113)



___
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] lavfi: add deshake_opencl filter

2019-07-24 Thread Mark Thompson
On 20/07/2019 00:19, Jarek Samic wrote:
> This filter is the subject of my GSoC project.
> 
> This is a video stabilization / deshake filter (name undetermined, feel free 
> to discuss) that uses feature
> point matching and RANSAC to determine a camera path, smooths the camera path 
> with a gaussian filter, and
> then applies the new path to the video.
> 
> There are a number of debug features that can be turned on (viewing point 
> matches, viewing transform
> details, viewing average kernel execution times). See the bottom of the file.
> 
> The filter is finished feature-wise and therefore ready for some review. 
> There are a few things left for
> me to do before this can be merged, though:
> 
> * Improve performance of the OpenCL kernels (in particular: harris_response 
> and match_descriptors)
> * See what I can do to improve the RANSAC model generation to reduce the 
> amount of jitter in the result
> * Clean up the few remaining TODOs
> 
> Just keep that in mind.
> 
> ---
>  libavfilter/Makefile|2 +
>  libavfilter/allfilters.c|1 +
>  libavfilter/opencl/deshake.cl   |  621 ++
>  libavfilter/opencl_source.h |1 +
>  libavfilter/vf_deshake_opencl.c | 1992 +++
>  5 files changed, 2617 insertions(+)
>  create mode 100644 libavfilter/opencl/deshake.cl
>  create mode 100644 libavfilter/vf_deshake_opencl.c

As already noted, it's missing the configure test.

> diff --git a/libavfilter/opencl/deshake.cl b/libavfilter/opencl/deshake.cl
> new file mode 100644
> index 00..737239f634
> --- /dev/null
> +++ b/libavfilter/opencl/deshake.cl
> @@ -0,0 +1,621 @@
> +/*
> + * Copyright (C) 2000, Intel Corporation, all rights reserved.
> + * Copyright (C) 2013, OpenCV Foundation, all rights reserved.
> + * Third party copyrights are property of their respective owners.
> + *
> + * Redistribution and use in source and binary forms, with or without 
> modification,
> + * are permitted provided that the following conditions are met:
> + *
> + *   * Redistribution's of source code must retain the above copyright 
> notice,
> + * this list of conditions and the following disclaimer.
> + *
> + *   * Redistribution's in binary form must reproduce the above copyright 
> notice,
> + * this list of conditions and the following disclaimer in the 
> documentation
> + * and/or other materials provided with the distribution.
> + *
> + *   * The name of the copyright holders may not be used to endorse or 
> promote products
> + * derived from this software without specific prior written permission.
> + *
> + * This software is provided by the copyright holders and contributors "as 
> is" and
> + * any express or implied warranties, including, but not limited to, the 
> implied
> + * warranties of merchantability and fitness for a particular purpose are 
> disclaimed.
> + * In no event shall the Intel Corporation or contributors be liable for any 
> direct,
> + * indirect, incidental, special, exemplary, or consequential damages
> + * (including, but not limited to, procurement of substitute goods or 
> services;
> + * loss of use, data, or profits; or business interruption) however caused
> + * and on any theory of liability, whether in contract, strict liability,
> + * or tort (including negligence or otherwise) arising in any way out of
> + * the use of this software, even if advised of the possibility of such 
> damage.
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
> USA
> + */

The way this is written makes it look like the whole file is usable under the 
3-clause BSD licence - I don't think that's what you intended?

I think I would either put the two copyright notices the other way around, or 
add copyright clause for yourself before the second licence.


Some random comments follow.  I've already been through this before, so most of 
this is odd minor stuff I'm happening to notice this time.

> +
> ...
> +
> +// Converts the src image to grayscale
> +__kernel void grayscale(
> +__read_only image2d_t src,
> +__write_only image2d_t grayscale
> +) {
> +int2 loc = (int2)(get_global_id(0), get_global_id(1));
> +write_imagef(grayscale, loc, (float4)(pixel_grayscale(src, loc), 

Re: [FFmpeg-devel] [PATCH 3/3] avcodec/utils: fix leak of subtitle_header on error path

2019-07-24 Thread Michael Niedermayer
On Fri, Jul 05, 2019 at 01:28:35AM +0200, Michael Niedermayer wrote:
> Fixes: memleak
> Fixes: 
> 15528/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_STL_fuzzer-5735993371525120
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/utils.c | 1 +
>  1 file changed, 1 insertion(+)

will apply with several additional cases mentioned in the commit message
as i found more cases that hit this same leak


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

Frequently ignored answer#1 FFmpeg bugs should be sent to our bugtracker. User
questions about the command line tools should be sent to the ffmpeg-user ML.
And questions about how to use libav* should be sent to the libav-user ML.


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 2/2] avformat/mpc: deallocate frames array on errors

2019-07-24 Thread Michael Niedermayer
Fixes: memleak on error path
Fixes: 
15984/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-5679918412726272

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer 
---
 libavformat/mpc.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/libavformat/mpc.c b/libavformat/mpc.c
index 487ff90c7d..a7b2e116ed 100644
--- a/libavformat/mpc.c
+++ b/libavformat/mpc.c
@@ -88,7 +88,7 @@ static int mpc_read_header(AVFormatContext *s)
 
 st = avformat_new_stream(s, NULL);
 if (!st)
-return AVERROR(ENOMEM);
+goto mem_error;
 st->codecpar->codec_type = AVMEDIA_TYPE_AUDIO;
 st->codecpar->codec_id = AV_CODEC_ID_MUSEPACK7;
 st->codecpar->channels = 2;
@@ -96,7 +96,7 @@ static int mpc_read_header(AVFormatContext *s)
 st->codecpar->bits_per_coded_sample = 16;
 
 if (ff_get_extradata(s, st->codecpar, s->pb, 16) < 0)
-return AVERROR(ENOMEM);
+goto mem_error;
 st->codecpar->sample_rate = mpc_rate[st->codecpar->extradata[2] & 3];
 avpriv_set_pts_info(st, 32, MPC_FRAMESIZE, st->codecpar->sample_rate);
 /* scan for seekpoints */
@@ -113,6 +113,9 @@ static int mpc_read_header(AVFormatContext *s)
 }
 
 return 0;
+mem_error:
+av_freep(>frames);
+return AVERROR(ENOMEM);
 }
 
 static int mpc_read_packet(AVFormatContext *s, AVPacket *pkt)
-- 
2.22.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".

[FFmpeg-devel] [PATCH 1/2] avcodec/assdec: undefined use of memcpy()

2019-07-24 Thread Michael Niedermayer
Fixes: null pointer passed as argument 2, which is declared to never be null
Fixes: 
16008/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SSA_fuzzer-5650582821404672
 (this is a separate issue found in this testcase)

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer 
---
 libavcodec/assdec.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libavcodec/assdec.c b/libavcodec/assdec.c
index 3178f2953c..f0b1069cd2 100644
--- a/libavcodec/assdec.c
+++ b/libavcodec/assdec.c
@@ -31,7 +31,8 @@ static av_cold int ass_decode_init(AVCodecContext *avctx)
 avctx->subtitle_header = av_malloc(avctx->extradata_size + 1);
 if (!avctx->subtitle_header)
 return AVERROR(ENOMEM);
-memcpy(avctx->subtitle_header, avctx->extradata, avctx->extradata_size);
+if (avctx->extradata_size)
+memcpy(avctx->subtitle_header, avctx->extradata, 
avctx->extradata_size);
 avctx->subtitle_header[avctx->extradata_size] = 0;
 avctx->subtitle_header_size = avctx->extradata_size;
 return 0;
-- 
2.22.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] Extract QP from h264 encoded videos

2019-07-24 Thread Juan De León
Previous thread:
http://ffmpeg.org/pipermail/ffmpeg-devel/2019-July/246951.html
I added the modifications to the decoder, I ran some tests for performance
and run times are only affected if my flag is enabled.
Decoded 3 different encoded videos 20 times each with and without my debug
flag, here are the results:

*ExtractQP disabled:*
===> multitime results
1: ffmpeg -hide_banner -loglevel panic -debug 0 -i tveryfast.mp4 -f null -
MeanStd.Dev.Min Median  Max
real0.747   0.007   0.735   0.749   0.756
user5.582   0.025   5.547   5.574   5.627
sys 0.166   0.028   0.120   0.167   0.224
===> multitime results
1: ffmpeg -hide_banner -loglevel panic -debug 0 -i tmedium.mp4 -f null -
MeanStd.Dev.Min Median  Max
real0.865   0.009   0.845   0.864   0.887
user6.296   0.036   6.198   6.299   6.365
sys 0.195   0.026   0.142   0.199   0.247
===> multitime results
1: ffmpeg -hide_banner -loglevel panic -debug 0 -i tveryslow.mp4 -f null -
MeanStd.Dev.Min Median  Max
real0.919   0.011   0.892   0.920   0.943
user6.398   0.042   6.311   6.381   6.476
sys 0.229   0.032   0.169   0.238   0.287

*ExtractQP enabled: *
===> multitime results
1: ffmpeg -hide_banner -loglevel panic -debug extractqp -i tveryfast.mp4 -f
null -
MeanStd.Dev.Min Median  Max
real1.126   0.032   1.076   1.132   1.216
user6.433   0.054   6.347   6.430   6.561
sys 1.069   0.047   0.989   1.063   1.161
===> multitime results
1: ffmpeg -hide_banner -loglevel panic -debug extractqp -i tmedium.mp4 -f
null -
MeanStd.Dev.Min Median  Max
real1.178   0.020   1.143   1.176   1.217
user7.091   0.055   7.020   7.081   7.196
sys 1.031   0.057   0.898   1.043   1.131
===> multitime results
1: ffmpeg -hide_banner -loglevel panic -debug extractqp -i tveryslow.mp4 -f
null -
MeanStd.Dev.Min Median  Max
real1.234   0.028   1.196   1.230   1.322
user7.212   0.077   6.996   7.230   7.345
sys 1.067   0.076   0.938   1.062   1.283
___
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] Extract QP from h264 encoded videos

2019-07-24 Thread Juan De León
---
 libavcodec/avcodec.h|   1 +
 libavcodec/h264dec.c|  37 
 libavcodec/options_table.h  |   1 +
 libavutil/Makefile  |   2 +
 libavutil/frame.h   |   6 ++
 libavutil/quantization_params.c |  40 +
 libavutil/quantization_params.h | 102 
 7 files changed, 189 insertions(+)
 create mode 100644 libavutil/quantization_params.c
 create mode 100644 libavutil/quantization_params.h

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index d234271c5b..9e3185720a 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -2671,6 +2671,7 @@ typedef struct AVCodecContext {
 #endif
 #define FF_DEBUG_BUFFERS 0x8000
 #define FF_DEBUG_THREADS 0x0001
+#define FF_DEBUG_EXTRACTQP   0x0002
 #define FF_DEBUG_GREEN_MD0x0080
 #define FF_DEBUG_NOMC0x0100
 
diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
index 8d1bd16a8e..07b85f4e0a 100644
--- a/libavcodec/h264dec.c
+++ b/libavcodec/h264dec.c
@@ -33,6 +33,7 @@
 #include "libavutil/opt.h"
 #include "libavutil/stereo3d.h"
 #include "libavutil/timer.h"
+#include "libavutil/quantization_params.h"
 #include "internal.h"
 #include "bytestream.h"
 #include "cabac.h"
@@ -922,6 +923,42 @@ static int finalize_frame(H264Context *h, AVFrame *dst, 
H264Picture *out, int *g
 }
 }
 
+if (h->avctx->debug & FF_DEBUG_EXTRACTQP) {
+int mb_height = h->height / 16;
+int mb_width = h->width / 16;
+int mb_xy = mb_width * mb_height;
+
+AVFrameSideData *sd;
+sd = av_frame_new_side_data(dst, AV_FRAME_DATA_QUANTIZATION_PARAMS,
+  sizeof(AVQuantizationParamsArray));
+
+AVQuantizationParamsArray *params;
+params = (AVQuantizationParamsArray *)sd->data;
+params->nb_blocks = mb_xy;
+params->qp_arr = av_malloc_array(mb_xy, sizeof(AVQuantizationParams));
+
+params->codec_id = h->avctx->codec_id;
+
+// loop allocate QP
+int qp_index = 0;
+for (int mb_y = 0; mb_y < mb_height; mb_y++) {
+for (int mb_x = 0; mb_x < mb_width; mb_x++) {
+int qs_index = mb_x + mb_y * h->mb_stride;
+AVQuantizationParams *qp_block = &(params->qp_arr[qp_index]);
+
+qp_block->x = mb_x * 16;
+qp_block->y = mb_y * 16;
+qp_block->w = qp_block->h = 16;
+
+// ALLOCATE MEMORY TO THE QP ARRAY
+qp_block->type = av_malloc(QP_TYPE_ARR_SIZE_H264 * 
sizeof(int));
+qp_block->type[QP_H264] = out->qscale_table[qs_index];
+
+qp_index++;
+}
+}
+}
+
 return 0;
 }
 
diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
index 4a266eca16..e0e78a69c5 100644
--- a/libavcodec/options_table.h
+++ b/libavcodec/options_table.h
@@ -219,6 +219,7 @@ static const AVOption avcodec_options[] = {
 {"buffers", "picture buffer allocations", 0, AV_OPT_TYPE_CONST, {.i64 = 
FF_DEBUG_BUFFERS }, INT_MIN, INT_MAX, V|D, "debug"},
 {"thread_ops", "threading operations", 0, AV_OPT_TYPE_CONST, {.i64 = 
FF_DEBUG_THREADS }, INT_MIN, INT_MAX, V|A|D, "debug"},
 {"nomc", "skip motion compensation", 0, AV_OPT_TYPE_CONST, {.i64 = 
FF_DEBUG_NOMC }, INT_MIN, INT_MAX, V|A|D, "debug"},
+{"extractqp", "enable QP extraction per frame", 0, AV_OPT_TYPE_CONST, {.i64 = 
FF_DEBUG_EXTRACTQP }, INT_MIN, INT_MAX, V|D, "debug"},
 {"dia_size", "diamond type & size for motion estimation", OFFSET(dia_size), 
AV_OPT_TYPE_INT, {.i64 = DEFAULT }, INT_MIN, INT_MAX, V|E},
 {"last_pred", "amount of motion predictors from the previous frame", 
OFFSET(last_predictor_count), AV_OPT_TYPE_INT, {.i64 = DEFAULT }, INT_MIN, 
INT_MAX, V|E},
 #if FF_API_PRIVATE_OPT
diff --git a/libavutil/Makefile b/libavutil/Makefile
index 8a7a44e4b5..be1a9c3a9c 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -60,6 +60,7 @@ HEADERS = adler32.h   
  \
   pixdesc.h \
   pixelutils.h  \
   pixfmt.h  \
+  quantization_params.h \
   random_seed.h \
   rc4.h \
   rational.h\
@@ -140,6 +141,7 @@ OBJS = adler32.o
\
parseutils.o \
pixdesc.o\
pixelutils.o \
+   quantization_params.o\

[FFmpeg-devel] [PATCH 6/6] h264_mp4toannexb_bsf: Try to avoid four byte startcodes

2019-07-24 Thread Andreas Rheinhardt
According to the H.264 specifications, the only NAL units that need to
have four byte startcodes in H.264 Annex B format are SPS/PPS units and
units that start a new access unit. Before af7e953a, the first of these
conditions wasn't upheld as already existing in-band parameter sets
would not automatically be written with a four byte startcode, but only
when they already were at the beginning of their input packets. But it
made four byte startcodes be used too often as every unit that is written
together with a parameter set that is inserted from extradata received a
four byte startcode although a three byte start code would suffice
unless the unit itself were a parameter set.

FATE has been updated to reflect the changes. Although the patch leaves
the extradata unchanged, the size of the extradata according to the FATE
reports changes. This is due to a quirk in ff_h2645_packet_split which
is used by extract_extradata: If the input is Annex B, the first zero of
a four byte startcode is considered a part of the last unit (if any).

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/h264_mp4toannexb_bsf.c|  6 +++---
 tests/ref/fate/h264-bsf-mp4toannexb  |  2 +-
 tests/ref/fate/h264_mp4toannexb_ticket2991   | 22 ++--
 tests/ref/fate/h264_mp4toannexb_ticket5927   | 10 -
 tests/ref/fate/h264_mp4toannexb_ticket5927_2 | 10 -
 tests/ref/fate/segment-mp4-to-ts |  4 ++--
 6 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/libavcodec/h264_mp4toannexb_bsf.c 
b/libavcodec/h264_mp4toannexb_bsf.c
index 5d9b71000e..caf690e3e2 100644
--- a/libavcodec/h264_mp4toannexb_bsf.c
+++ b/libavcodec/h264_mp4toannexb_bsf.c
@@ -44,7 +44,7 @@ static int alloc_and_copy(AVPacket *out,
   const uint8_t *in, uint32_t in_size, int ps)
 {
 uint32_t offset = out->size;
-uint8_t start_code_size = offset == 0 || ps ? 4 : 3;
+uint8_t start_code_size = offset == 0 && sps_pps_size == 0 || ps ? 4 : 3;
 int err;
 
 err = av_grow_packet(out, sps_pps_size + in_size + start_code_size);
@@ -238,7 +238,7 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx, 
AVPacket *out)
 if (s->new_idr && unit_type == H264_NAL_IDR_SLICE && !s->idr_sps_seen 
&& !s->idr_pps_seen) {
 if ((ret=alloc_and_copy(out,
ctx->par_out->extradata, 
ctx->par_out->extradata_size,
-   buf, nal_size, 1)) < 0)
+   buf, nal_size, 0)) < 0)
 goto fail;
 s->new_idr = 0;
 /* if only SPS has been seen, also insert PPS */
@@ -249,7 +249,7 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx, 
AVPacket *out)
 goto fail;
 } else if ((ret = alloc_and_copy(out,
 ctx->par_out->extradata + 
s->pps_offset, ctx->par_out->extradata_size - s->pps_offset,
-buf, nal_size, 1)) < 0)
+buf, nal_size, 0)) < 0)
 goto fail;
 } else {
 if ((ret=alloc_and_copy(out, NULL, 0, buf, nal_size, unit_type == 
H264_NAL_SPS || unit_type == H264_NAL_PPS)) < 0)
diff --git a/tests/ref/fate/h264-bsf-mp4toannexb 
b/tests/ref/fate/h264-bsf-mp4toannexb
index 7cd086a268..2049f39701 100644
--- a/tests/ref/fate/h264-bsf-mp4toannexb
+++ b/tests/ref/fate/h264-bsf-mp4toannexb
@@ -1 +1 @@
-f340e7ca9a46d437af4e96f6c8de221c
+5f04c27cc6ee8625fe2405fb0f7da9a3
diff --git a/tests/ref/fate/h264_mp4toannexb_ticket2991 
b/tests/ref/fate/h264_mp4toannexb_ticket2991
index 3245ef442c..76bdf3cae7 100644
--- a/tests/ref/fate/h264_mp4toannexb_ticket2991
+++ b/tests/ref/fate/h264_mp4toannexb_ticket2991
@@ -1,12 +1,12 @@
-dba672c154b41414cf26aae967c27eef 
*tests/data/fate/h264_mp4toannexb_ticket2991.h264
-1985823 tests/data/fate/h264_mp4toannexb_ticket2991.h264
-#extradata 0:   48, 0x47ae0d55
+05d66e60ab22ee004720e0051af0fe74 
*tests/data/fate/h264_mp4toannexb_ticket2991.h264
+1985815 tests/data/fate/h264_mp4toannexb_ticket2991.h264
+#extradata 0:   47, 0x3a590d55
 #tb 0: 1/120
 #media_type 0: video
 #codec_id 0: h264
 #dimensions 0: 1280x720
 #sar 0: 3/4
-0,  0,  0,48000,37127, 0xc125184c
+0,  0,  0,48000,37126, 0xb020184c
 0,  48000,  48000,40040, 6920, 0x8512361a, F=0x0
 0,  88040,  88040,40040, 7550, 0x1bc56ed4, F=0x0
 0, 128081, 128081,40040, 8752, 0xb8c6f0a1, F=0x0
@@ -21,7 +21,7 @@ dba672c154b41414cf26aae967c27eef 
*tests/data/fate/h264_mp4toannexb_ticket2991.h2
 0, 488444, 488444,40040,11234, 0x83cbd9fd, F=0x0
 0, 528485, 528485,40040,17616, 0xfdf95104, F=0x0
 0, 568525, 568525,40040,10689, 0x9633d32b, F=0x0
-0, 608566, 608566,40040,45292, 0x66dd2cf6
+0, 608566, 608566,40040,45291, 0x543c2cf6
 0, 

[FFmpeg-devel] [PATCH 4/6] h264_mp4toannexb_bsf: Don't forget numOfPictureParameterSets

2019-07-24 Thread Andreas Rheinhardt
The format of an AVCDecoderConfigurationRecord, the out-of-band
extradata of H.264 in mp4, is as follows: First four bytes containing
version, profile and level, one byte for the length size and one byte
each for the number of SPS, followed by the SPS (each with its own size
field), followed by a byte containing the number of PPS followed by the
PPS with their size fields. While the number of SPS/PPS may be zero, the
bytes containing these numbers are mandatory. Yet the byte containing
the number of PPS has been ignored in two places:
1. In the initial check for whether the extradata can contain an
AVCDecoderConfigurationRecord. The minimum size is 7, not 6.
2. No check is made for whether the extradata ended right after the last
byte of the last SPS of the SPS array. Instead the first byte of the
padding is read as if it were part of the extradata and contained the
number of PPS (namely zero, given that the padding is zeroed). No error
or warning was ever raised.
This has been changed. Such truncated extradata is now considered
invalid; the check for 2. has been incorporated into the general size
check.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/h264_mp4toannexb_bsf.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libavcodec/h264_mp4toannexb_bsf.c 
b/libavcodec/h264_mp4toannexb_bsf.c
index aa5ca8d102..0f46ad907c 100644
--- a/libavcodec/h264_mp4toannexb_bsf.c
+++ b/libavcodec/h264_mp4toannexb_bsf.c
@@ -95,8 +95,8 @@ static int h264_extradata_to_annexb(AVBSFContext *ctx, const 
int padding)
 extradata  += 2;
 total_size += unit_size + 4;
 av_assert1(total_size <= INT_MAX - padding);
-if (extradata_end - extradata < unit_size) {
-av_log(ctx, AV_LOG_ERROR, "Packet header is not contained in 
global extradata, "
+if (extradata_end - extradata < unit_size + !sps_done) {
+av_log(ctx, AV_LOG_ERROR, "Global extradata truncated, "
"corrupted stream or invalid MP4/AVCC bitstream\n");
 av_free(out);
 return AVERROR(EINVAL);
@@ -148,7 +148,7 @@ static int h264_mp4toannexb_init(AVBSFContext *ctx)
 (extra_size >= 4 && AV_RB32(ctx->par_in->extradata) == 1)) {
 av_log(ctx, AV_LOG_VERBOSE,
"The input looks like it is Annex B already\n");
-} else if (extra_size >= 6) {
+} else if (extra_size >= 7) {
 ret = h264_extradata_to_annexb(ctx, AV_INPUT_BUFFER_PADDING_SIZE);
 if (ret < 0)
 return ret;
-- 
2.21.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".

[FFmpeg-devel] [PATCH 5/6] h264_mp4toannexb: Improve overread checks

2019-07-24 Thread Andreas Rheinhardt
1. Left shifts of negative values are undefined as soon as the sign bit
is involved. Therefore make nal_size an uint32_t and drop the check for
whether it is < 0.
2. The two checks for overreads (whether the length field is contained
in the packet and whether the actual unit is contained in the packet)
can be combined into one because the packet is padded, i.e. a potential
overread caused by reading the length field without checking whether
said length field is actually part of the packet's buffer is allowed
as one always stays within the padding. But one has to be aware of
a pitfall: The comparison must be performed in (at least) int64_t as
otherwise buf_end - buf might be promoted to uint32_t in which case
an already occured overread would appear as a very large number.
A comment explaining this has been added, too.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/h264_mp4toannexb_bsf.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/libavcodec/h264_mp4toannexb_bsf.c 
b/libavcodec/h264_mp4toannexb_bsf.c
index 0f46ad907c..5d9b71000e 100644
--- a/libavcodec/h264_mp4toannexb_bsf.c
+++ b/libavcodec/h264_mp4toannexb_bsf.c
@@ -172,8 +172,7 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx, 
AVPacket *out)
 
 AVPacket *in;
 uint8_t unit_type;
-int32_t nal_size;
-uint32_t cumul_size= 0;
+uint32_t cumul_size = 0, nal_size;
 const uint8_t *buf;
 const uint8_t *buf_end;
 intbuf_size;
@@ -195,18 +194,19 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx, 
AVPacket *out)
 buf_end  = in->data + in->size;
 
 do {
-ret= AVERROR(EINVAL);
-if (buf + s->length_size > buf_end)
-goto fail;
-
+/* possible overread ok due to padding */
 for (nal_size = 0, i = 0; ilength_size; i++)
 nal_size = (nal_size << 8) | buf[i];
 
 buf += s->length_size;
 unit_type = *buf & 0x1f;
 
-if (nal_size > buf_end - buf || nal_size < 0)
+/* This check requires the cast as the right side might
+ * otherwise be promoted to an unsigned value. */
+if ((int64_t)nal_size > buf_end - buf) {
+ret = AVERROR(EINVAL);
 goto fail;
+}
 
 if (unit_type == H264_NAL_SPS)
 s->idr_sps_seen = s->new_idr = 1;
-- 
2.21.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".

[FFmpeg-devel] [PATCH 3/6] h264_mp4toannexb_bsf: Add a comment about possible overread

2019-07-24 Thread Andreas Rheinhardt
Before reading a 16bit size field during parsing of extradata, no check
is performed to make sure that said length field is actually contained
in the extradata. Given that this overread is not dangerous (the extradata
is supposed to be padded), only a comment for it has been added; the error
itself will be detected as part of the normal check for overreads.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/h264_mp4toannexb_bsf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/h264_mp4toannexb_bsf.c 
b/libavcodec/h264_mp4toannexb_bsf.c
index 374c2d59fb..aa5ca8d102 100644
--- a/libavcodec/h264_mp4toannexb_bsf.c
+++ b/libavcodec/h264_mp4toannexb_bsf.c
@@ -91,7 +91,7 @@ static int h264_extradata_to_annexb(AVBSFContext *ctx, const 
int padding)
 while (unit_nb--) {
 int err;
 
-unit_size   = AV_RB16(extradata);
+unit_size   = AV_RB16(extradata); /* possible overread ok due to 
padding */
 extradata  += 2;
 total_size += unit_size + 4;
 av_assert1(total_size <= INT_MAX - padding);
-- 
2.21.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".

[FFmpeg-devel] [PATCH 2/6] h264_mp4toannexb_bsf: Improve extradata overread checks

2019-07-24 Thread Andreas Rheinhardt
Currently during parsing the extradata, h264_mp4toannexb checks for
overreads by adding the size of the current unit to the current position
pointer and comparing this to the end position of the extradata. But
pointer comparisons and pointer arithmetic is only defined if it does not
exceed the object it is used on (one past the last element of an array
is allowed, too). In practice, this might lead to overflows. Therefore
the check has been changed.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/h264_mp4toannexb_bsf.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/libavcodec/h264_mp4toannexb_bsf.c 
b/libavcodec/h264_mp4toannexb_bsf.c
index bbf124ad04..374c2d59fb 100644
--- a/libavcodec/h264_mp4toannexb_bsf.c
+++ b/libavcodec/h264_mp4toannexb_bsf.c
@@ -72,7 +72,8 @@ static int h264_extradata_to_annexb(AVBSFContext *ctx, const 
int padding)
 uint32_t total_size = 0;
 uint8_t *out= NULL, unit_nb, sps_done = 0,
  sps_seen   = 0, pps_seen = 0;
-const uint8_t *extradata= ctx->par_in->extradata + 4;
+const uint8_t *extradata= ctx->par_in->extradata + 4,
+  *extradata_end= ctx->par_in->extradata + 
ctx->par_in->extradata_size;
 static const uint8_t nalu_header[4] = { 0, 0, 0, 1 };
 int length_size = (*extradata++ & 0x3) + 1; // retrieve length coded size
 
@@ -91,9 +92,10 @@ static int h264_extradata_to_annexb(AVBSFContext *ctx, const 
int padding)
 int err;
 
 unit_size   = AV_RB16(extradata);
+extradata  += 2;
 total_size += unit_size + 4;
 av_assert1(total_size <= INT_MAX - padding);
-if (extradata + 2 + unit_size > ctx->par_in->extradata + 
ctx->par_in->extradata_size) {
+if (extradata_end - extradata < unit_size) {
 av_log(ctx, AV_LOG_ERROR, "Packet header is not contained in 
global extradata, "
"corrupted stream or invalid MP4/AVCC bitstream\n");
 av_free(out);
@@ -102,8 +104,8 @@ static int h264_extradata_to_annexb(AVBSFContext *ctx, 
const int padding)
 if ((err = av_reallocp(, total_size + padding)) < 0)
 return err;
 memcpy(out + total_size - unit_size - 4, nalu_header, 4);
-memcpy(out + total_size - unit_size, extradata + 2, unit_size);
-extradata += 2 + unit_size;
+memcpy(out + total_size - unit_size, extradata, unit_size);
+extradata += unit_size;
 pps:
 if (!unit_nb && !sps_done++) {
 unit_nb = *extradata++; /* number of pps unit(s) */
-- 
2.21.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] avcodec/rl2: set dimensions

2019-07-24 Thread James Almer
On 7/23/2019 9:39 PM, Kieran Kunhya wrote:
>>
>> What was the cause of the slow decoding? Does this actually fix it, or
>> does it just swipe it "under the carpet"?
>> If someone ever found a sample with a different solution, how would they
>> know that they shouldn't just remove this again? Without any kind of
>> comment on the point of this call, people might assume it's pointless
>> nonsense.
>>
> 
> A significant proportion of these patches sweep the issue under the carpet.
> Not to mention the swathes of annoyed developers
> 
> Kieran

The only very vocal people so far that i could see have been you and
Lynne. Everyone else at most didn't find these patches useful, but Lynne
especially has been very aggressive to communicate their displeasure
about fuzzing related fixes, and unjustifiably so.
If some of these patches affect code you're interested into, then
suggest nicer looking alternatives, like i did for the recent mpc8 patch.

Patches like this one that fixes timeouts during fuzzing are usually
validation checks to abort on invalid data early, and in this case they
actually force an spec constrain in the correct place. They are hardly
clutter. And if they use what you consider magic numbers or heuristics,
or "sweep the issue under the carpet", you can always just request a
comment to be placed next to them, like Reimar just did, or use some
configurable value, like the recently added
AVCodecContext.discard_damaged_percentage

My point is, instead of complaining about UB fixes, try to be
constructive about how to implement them better. Campaigning to migrate
the codebase to something else than C and cut the issue from the root is
more productive than replying to every other fuzzing fix like this.
___
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] avfilter/vf_convolution: add x86 SIMD for filter_3x3()

2019-07-24 Thread Paul B Mahol
On 7/17/19, Paul B Mahol  wrote:
> On 7/15/19, Song, Ruiling  wrote:
>>> -Original Message-
>>> From: Song, Ruiling
>>> Sent: Tuesday, July 9, 2019 9:15 AM
>>> To: ffmpeg-devel@ffmpeg.org
>>> Cc: Song, Ruiling 
>>> Subject: [PATCH] avfilter/vf_convolution: add x86 SIMD for filter_3x3()
>>>
>>> Tested using a simple command (apply edge enhance):
>>> ./ffmpeg_g -i ~/Downloads/bbb_sunflower_1080p_30fps_normal.mp4 \
>>>  -vf convolution="0 0 0 -1 1 0 0 0 0:0 0 0 -1 1 0 0 0 0:0 0 0 -1 1 0 0 0
>>> 0:0 0 0 -1 1 0 0
>>> 0 0:5:1:1:1:0:128:128:128" \
>>>  -an -vframes 1000 -f null /dev/null
>>>
>>> The fps increase from 151 to 270 on my local machine.
>>>
>>> Signed-off-by: Ruiling Song 
>> Ping?
>
> Should be fine IFF output is exact with C version (under different
> parameters).
>

So can you confirm this?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/4] h264_mp4toannexb_bsf: Improve extradata overread checks

2019-07-24 Thread Andreas Rheinhardt
Michael Niedermayer:
> On Mon, Jul 22, 2019 at 05:27:13AM +0200, Andreas Rheinhardt wrote:
>> 1. Currently during parsing the extradata, h264_mp4toannexb checks for
>> overreads by adding the size of the current unit to the current position
>> pointer and comparing this to the end position of the extradata. But
>> pointer comparisons and pointer arithmetic is only defined if it does not
>> exceed the object it is used on (one past the last element of an array
>> is allowed, too). In practice, this might lead to overflows. Therefore
>> the check has been changed.
>> 2. The minimal size of an AVCDecoderConfigurationRecord is actually 7:
>> Four bytes containing version, profile and level, one byte for length
>> size and one byte each for the numbers of SPS and PPS. This has been
>> changed. The byte for the number of PPS has been forgotten.
> 
>> 3. The earlier code also did not detect an error if the extradata ended
>> directly after the last SPS in the SPS array (if any) although the
>> number of PPS has to come afterwards. A check for this has been
>> integrated into the general overread check.
> 
> what if there is no pps afterwards and instead in stream?
> could this change break such streams ?
> 
This patch increases the strictness for exactly one scenario: If the
extradata immediately ends after the SPS array, without the
numOfPictureParameterSets byte (that has to be present). This is now
considered invalid data, earlier code read into the padding and
therefore concluded that the number of PPS is zero. If the byte is
present and zero, then nothing changes: the stream will be playable if
the PPS is available in-band.
> 
>> 4. The earlier code also might overread when reading the size of the
>> next unit. Given that this overread is not dangerous (the extradata is
>> supposed to be padded), only a comment for it has been added; the error
>> itself will be detected as part of the normal check for overreads.
>>
>> Signed-off-by: Andreas Rheinhardt 
>> ---
>>  libavcodec/h264_mp4toannexb_bsf.c | 16 +---
>>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> changes feel ok but this should be split as they are independant issues
> 
Ok, will split into 1. + (2. + 3.) + 4.

- Andreas
___
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] h264_mp4toannexb: Remove unnecessary check

2019-07-24 Thread Andreas Rheinhardt
There can be at most 31 SPS and 255 PPS in the mp4/Matroska extradata.
Given that each has a size of at most 2^16-1, the length of the output
derived from these parameter sets can never overflow an ordinary 32 bit
integer. So use a simple uint32_t instead of uint64_t and replace the
unnecessary check with an av_assert1.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/h264_mp4toannexb_bsf.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/libavcodec/h264_mp4toannexb_bsf.c 
b/libavcodec/h264_mp4toannexb_bsf.c
index fb3f24ea40..bbf124ad04 100644
--- a/libavcodec/h264_mp4toannexb_bsf.c
+++ b/libavcodec/h264_mp4toannexb_bsf.c
@@ -21,6 +21,7 @@
 
 #include 
 
+#include "libavutil/avassert.h"
 #include "libavutil/intreadwrite.h"
 #include "libavutil/mem.h"
 
@@ -68,7 +69,7 @@ static int h264_extradata_to_annexb(AVBSFContext *ctx, const 
int padding)
 {
 H264BSFContext *s = ctx->priv_data;
 uint16_t unit_size;
-uint64_t total_size = 0;
+uint32_t total_size = 0;
 uint8_t *out= NULL, unit_nb, sps_done = 0,
  sps_seen   = 0, pps_seen = 0;
 const uint8_t *extradata= ctx->par_in->extradata + 4;
@@ -91,12 +92,7 @@ static int h264_extradata_to_annexb(AVBSFContext *ctx, const 
int padding)
 
 unit_size   = AV_RB16(extradata);
 total_size += unit_size + 4;
-if (total_size > INT_MAX - padding) {
-av_log(ctx, AV_LOG_ERROR,
-   "Too big extradata size, corrupted stream or invalid 
MP4/AVCC bitstream\n");
-av_free(out);
-return AVERROR(EINVAL);
-}
+av_assert1(total_size <= INT_MAX - padding);
 if (extradata + 2 + unit_size > ctx->par_in->extradata + 
ctx->par_in->extradata_size) {
 av_log(ctx, AV_LOG_ERROR, "Packet header is not contained in 
global extradata, "
"corrupted stream or invalid MP4/AVCC bitstream\n");
-- 
2.21.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] how to create sub project / git repo under FFMPEG

2019-07-24 Thread Moritz Barsnick
On Tue, Jul 23, 2019 at 13:49:30 +, Guo, Yejun wrote:
> > Actually, http://git.videolan.org/?p=ffmpeg.git;a=summary is the
> > official repo (or rather its visualization). Github is the secondary
> > repo, a mirror.
>
> thanks Moritz, do you mean that the official repo is not 
> https://git.ffmpeg.org/ffmpeg.git
> that I mentioned, but http://git.videolan.org/?p=ffmpeg.git;a=summary?
>
> I got the git repo at https://ffmpeg.org/download.html#get-sources.

Sorry, I mixed that up. I don't know where I got Github into the
equation.

In addition to what I wrote about Github vs. VideoLAN: source.ffmpeg.org
is just an alias to git.videolan.org. That doesn't seem to be the case
for git.ffmpeg.org. So I actually don't know what is "official", and
what the reference.

Moritz
___
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] vp3data, mpc8huff: Make some arrays unsigned to prevent overflow

2019-07-24 Thread Michael Niedermayer
On Sun, Jul 21, 2019 at 02:07:07PM +1000, Peter Ross wrote:
> On Sat, Jul 20, 2019 at 03:51:25PM +0200, Andreas Rheinhardt wrote:
> > Some of the VP3 arrays (namely vp31_intra_y_dequant, vp31_intra_c_dequant
> > and vp31_inter_dequant) are currently declared as array of (const) int8_t
> > despite them being only used to directly initialize an array of uint8_t.
> > vp31_inter_dequant even contains the value 128 which is not
> > representible in int8_t and might generate overflow warnings by
> > compilers.
> > 
> > Similarly, mpc8_q4_syms is an array of int8_t that is initialized using
> > values not in the range of an int8_t and that is only accessed via
> > a pointer to uint8_t in ff_init_vlc_sparse. The latter applies to all
> > the other *_bits and *_syms tables in mpc8huff.h, so make them all
> > unsigned.
> > 
> > Signed-off-by: Andreas Rheinhardt 
> > ---
> >  libavcodec/mpc8huff.h | 34 +-
> >  libavcodec/vp3data.h  |  6 +++---
> >  2 files changed, 20 insertions(+), 20 deletions(-)
> > 
> > diff --git a/libavcodec/mpc8huff.h b/libavcodec/mpc8huff.h
> > index 8491037aa4..0566c910ca 100644
> > --- a/libavcodec/mpc8huff.h
> > +++ b/libavcodec/mpc8huff.h
> > @@ -34,7 +34,7 @@ static const uint8_t mpc8_bands_codes[MPC8_BANDS_SIZE] = {
> >   0x08, 0x09, 0x06, 0x07, 0x05, 0x05, 0x03, 0x03,
> >   0x01,
> >  };
> > -static const int8_t mpc8_bands_bits[MPC8_BANDS_SIZE] = {
> > +static const uint8_t mpc8_bands_bits[MPC8_BANDS_SIZE] = {
> >1,  3,  5,  6,  7,  8,  8,  9,
> >   10, 11, 12, 12, 12, 13, 12, 12,
> >   12, 12, 12, 13, 12, 12, 12, 11,
> > @@ -48,7 +48,7 @@ static const int8_t mpc8_bands_bits[MPC8_BANDS_SIZE] = {
> >  static const uint8_t mpc8_scfi0_codes[MPC8_SCFI0_SIZE] = {
> >   0x00, 0x01, 0x01, 0x01,
> >  };
> > -static const int8_t mpc8_scfi0_bits[MPC8_SCFI0_SIZE] = {
> > +static const uint8_t mpc8_scfi0_bits[MPC8_SCFI0_SIZE] = {
> >3,  3,  1,  2,
> >  };
> >  
> > @@ -60,7 +60,7 @@ static const uint8_t mpc8_scfi1_codes[MPC8_SCFI1_SIZE] = {
> >   0x04, 0x06, 0x02, 0x02, 0x05, 0x07, 0x03, 0x03,
> >  
> >  };
> > -static const int8_t mpc8_scfi1_bits[MPC8_SCFI1_SIZE] = {
> > +static const uint8_t mpc8_scfi1_bits[MPC8_SCFI1_SIZE] = {
> >6,  7,  6,  6,  7,  5,  5,  5,
> >6,  5,  2,  3,  6,  5,  3,  2,
> >  
> > @@ -79,7 +79,7 @@ static const uint8_t mpc8_dscf0_codes[MPC8_DSCF0_SIZE] = {
> >   0x0C, 0x0D, 0x07, 0x08, 0x09, 0x06, 0x07, 0x03,
> >   0x04, 0x05, 0x00, 0x01, 0x02, 0x03, 0x04, 0x05,
> >  };
> > -static const int8_t mpc8_dscf0_bits[MPC8_DSCF0_SIZE] = {
> > +static const uint8_t mpc8_dscf0_bits[MPC8_DSCF0_SIZE] = {
> >   12, 12, 12, 11, 11, 11, 10, 10,
> >   10, 10, 10,  9,  9,  9,  9,  8,
> >8,  8,  8,  7,  7,  7,  7,  6,
> > @@ -105,7 +105,7 @@ static const uint8_t mpc8_dscf1_codes[MPC8_DSCF1_SIZE] 
> > = {
> >   0x05, 0x06, 0x07, 0x01, 0x02, 0x03, 0x04, 0x05,
> >   0x0D,
> >  };
> > -static const int8_t mpc8_dscf1_bits[MPC8_DSCF1_SIZE] = {
> > +static const uint8_t mpc8_dscf1_bits[MPC8_DSCF1_SIZE] = {
> >   15, 14, 14, 13, 13, 13, 12, 12,
> >   12, 12, 11, 11, 11, 11, 10, 10,
> >   10, 10,  9,  9,  9,  8,  8,  7,
> > @@ -132,7 +132,7 @@ static const uint8_t mpc8_res_codes[2][MPC8_RES_SIZE] = 
> > {
> >  0x03,
> >}
> >  };
> > -static const int8_t mpc8_res_bits[2][MPC8_RES_SIZE] = {
> > +static const uint8_t mpc8_res_bits[2][MPC8_RES_SIZE] = {
> >{
> >   1,  2,  4,  5,  6,  7,  9, 10,
> >  11, 12, 13, 14, 15, 16, 16,  8,
> > @@ -153,7 +153,7 @@ static const uint8_t mpc8_q1_codes[MPC8_Q1_SIZE] = {
> >   0x03, 0x04, 0x05, 0x01, 0x01, 0x01, 0x01, 0x01,
> >   0x01, 0x00, 0x01,
> >  };
> > -static const int8_t mpc8_q1_bits[MPC8_Q1_SIZE] = {
> > +static const uint8_t mpc8_q1_bits[MPC8_Q1_SIZE] = {
> >6,  4,  4,  3,  3,  3,  3,  3,
> >4,  4,  4,  5,  7,  8,  9, 10,
> >   11, 12, 12,
> > @@ -196,7 +196,7 @@ static const uint8_t mpc8_q9up_codes[MPC8_Q9UP_SIZE] = {
> >   0x42, 0x43, 0x44, 0x45, 0x46, 0x47, 0x48, 0x49,
> >   0x4A, 0x4B, 0x06, 0x07, 0x08, 0x09, 0x00, 0x01,
> >  };
> > -static const int8_t mpc8_q9up_bits[MPC8_Q9UP_SIZE] = {
> > +static const uint8_t mpc8_q9up_bits[MPC8_Q9UP_SIZE] = {
> >   10, 10, 10, 10, 10,  9,  9,  9,
> >9,  9,  9,  9,  9,  9,  9,  9,
> >9,  9,  9,  8,  8,  9,  9,  9,
> > @@ -272,7 +272,7 @@ static const uint8_t mpc8_q2_codes[2][MPC8_Q2_SIZE] = {
> >   0x03, 0x1C, 0x17, 0x1D, 0x05,
> >  }
> >  };
> > -static const int8_t mpc8_q2_bits[2][MPC8_Q2_SIZE] = {
> > +static const uint8_t mpc8_q2_bits[2][MPC8_Q2_SIZE] = {
> >  {
> >   12, 11, 10, 11, 13, 11,  9,  8,
> >9, 11, 11,  8,  7,  8, 11, 11,
> > @@ -324,7 +324,7 @@ static const uint8_t mpc8_q3_codes[MPC8_Q3_SIZE] = {
> >   0x06, 0x05, 0x04, 0x03, 0x02, 0x03, 0x02, 0x01,
> >   0x00,
> >  };
> > -static const int8_t mpc8_q3_bits[MPC8_Q3_SIZE] = {
> > +static const uint8_t mpc8_q3_bits[MPC8_Q3_SIZE] = {
> >3,  4,  4,  4,  4,  4,  4,  5,
> >5,  5,  5,  5,  5,  6,  6,  6,
> >6,  6,  6,  6,  6,  6,  6,  6,
> > @@ -333,7 

Re: [FFmpeg-devel] [PATCH, v3] fftools/ffmpeg_filter: add -autoscale to disable/enable the default scale

2019-07-24 Thread Paul B Mahol
On 7/24/19, Fu, Linjie  wrote:
>> -Original Message-
>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
>> Of Paul B Mahol
>> Sent: Wednesday, July 24, 2019 17:42
>> To: FFmpeg development discussions and patches > de...@ffmpeg.org>
>> Subject: Re: [FFmpeg-devel] [PATCH, v3] fftools/ffmpeg_filter: add -
>> autoscale to disable/enable the default scale
>>
>> On 7/24/19, Fu, Linjie  wrote:
>> >> -Original Message-
>> >> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
>> Behalf
>> >> Of Gyan
>> >> Sent: Wednesday, July 24, 2019 12:28
>> >> To: ffmpeg-devel@ffmpeg.org
>> >> Subject: Re: [FFmpeg-devel] [PATCH, v3] fftools/ffmpeg_filter: add -
>> >> autoscale to disable/enable the default scale
>> >>
>> >>
>> >>
>> >> On 24-07-2019 08:15 AM, Fu, Linjie wrote:
>> >> >> -Original Message-
>> >> >> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
>> >> Behalf
>> >> >> Of Gyan
>> >> >> Sent: Wednesday, July 24, 2019 00:49
>> >> >> To: ffmpeg-devel@ffmpeg.org
>> >> >> Subject: Re: [FFmpeg-devel] [PATCH, v3] fftools/ffmpeg_filter: add
>> >> >> -
>> >> >> autoscale to disable/enable the default scale
>> >> >>
>> >> >>
>> >> >>
>> >> >> On 22-07-2019 01:40 PM, Fu, Linjie wrote:
>> >>  -Original Message-
>> >>  From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org]
>> On
>> >> >> Behalf
>> >>  Of Gyan
>> >>  Sent: Saturday, July 20, 2019 13:29
>> >>  To: ffmpeg-devel@ffmpeg.org
>> >>  Subject: Re: [FFmpeg-devel] [PATCH, v3] fftools/ffmpeg_filter:
>> >>  add
>> -
>> >>  autoscale to disable/enable the default scale
>> >> > diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
>> >> > index cd35eb49c8..99121b6981 100644
>> >> > --- a/doc/ffmpeg.texi
>> >> > +++ b/doc/ffmpeg.texi
>> >> > +@item -autoscale
>> >> > +Automatically scale the video according to the resolution of
>> >> > first
>> >> frame.
>> >> > +Enabled by default, use @option{-noautoscale} to disable it.
>> When
>> >>  autoscale is
>> >> > +disabled, all output frames might not be in the same resolution
>> >> > and
>> >> >> may
>> >>  require
>> >> > +some additional explicit processing according to your final
>> >>  rendering/output
>> >> > +destination. Disabling autoscale may not work in all
>> >> > situations.
>> >> >> Therefore,
>> >>  it
>> >> > +is not recommended to disable it unless you really know what
>> you
>> >> are
>> >>  doing.
>> >> > +Disable autoscale at your own risk.
>> >>  Since the auto scaling happens at the end of the graph, what may
>> the
>> >>  "additional explicit processing" be?
>> >> >>> Vpp processing may not be influenced,  a warning in transcode is
>> >> >>> needed.
>> >> >>> The expression seems to be improper, how about:
>> >> >>>
>> >> >>> "When autoscale is disabled, all output frames of filter graph
>> >> >>> might
>> >> >>> not
>> >> be
>> >> >> in the same
>> >> >>> resolution and may be inadequate for encoder/muxer."
>> >> >>>
>> >> >>> or other suggestions?
>> >> >>>
>> >> > @@ -3640,6 +3642,12 @@ const OptionDef options[] = {
>> >> > { "autorotate",   HAS_ARG | OPT_BOOL | OPT_SPEC |
>> >> >   OPT_EXPERT | OPT_INPUT,
>> >> > { .off =
>> >>  OFFSET(autorotate) },
>> >> > "automatically insert correct rotate filters" },
>> >> > +{ "autoscale",HAS_ARG | OPT_BOOL | OPT_SPEC |
>> >> > +  OPT_EXPERT | OPT_INPUT,
>> >> >  { .off =
>> >>  OFFSET(autoscale) },
>> >> > +"automatically insert a scale filter at the end of the
>> >> > filter graph if
>> >> a
>> >>  resolution"
>> >> > +"change is detected. This ensures all frames are the
>> >> > same
>> >> >> resolution
>> >>  as the first frame"
>> >> > +"when they leave the filter chain (this option is
>> >> > enabled
>> >> > by
>> >> >> default)."
>> >> > +"If disabled, some encoders/muxers may not support this
>> >> mode."},
>> >>  Which muxers can detect or check for prop changes within coded
>> >>  bitstreams? Which encoders are known to be able to handle
>> >>  changing resolution?
>> >> >>> It's not supported currently (even in libvpx-vp9, since vp9
>> >> >>> supports
>> >> >> dynamic resolution in spec).
>> >> >>> I don't intend to be so absolutely in doc,  will it be better for
>> >> >>> you
>> >> >>> to
>> >> modify
>> >> >> like:
>> >> >>> "If disabled, encoders/muxers won't support this mode currently."
>> >> >> So other than`-c:v rawvideo -f rawvideo`, there is no
>> >> >> combination
>> >> >> that supports changing frame sizes?
>> >> > I didn't check all encoders.
>> >> > But since the resolution changing is detectable, some additional
>> >> > reinit
>> >> procedures
>> >> > can be added to change resolution at IDR frame and 

Re: [FFmpeg-devel] [PATCH] lavc/phtread_frame: update hwaccel_priv_data in time for multithread

2019-07-24 Thread Fu, Linjie
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Fu, Linjie
> Sent: Saturday, July 20, 2019 00:32
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] lavc/phtread_frame: update
> hwaccel_priv_data in time for multithread
> 
> > -Original Message-
> > From: Fu, Linjie
> > Sent: Friday, July 19, 2019 05:35
> > To: ffmpeg-devel@ffmpeg.org
> > Cc: Fu, Linjie 
> > Subject: [PATCH] lavc/phtread_frame: update hwaccel_priv_data in time
> for
> > multithread
> >
> > When resolution/format changes, hwaccel_uninit/hwaccel_init will
> > be called to destroy and re-create the hwaccel_priv_data. When output
> > frame number meets the constraints for vframes, the hwaccel_priv_data
> > modified in decoding thread won't be update to user-thread due to the
> > delay mechanism. It will lead to:
> > 1. memory leak in child-thread.
> > 2. double free in user-thread while calling avcodec_close().
> >
> > Can be reproduced with a resolution change case, and use -vframes
> > to terminate the decode during dynamic resolution changing:
> >
> > ffmpeg -hwaccel vaapi -hwaccel_device /dev/dri/renderD128 -v verbose
> > -i ./reinit-large_420_8-to-small_420_8.h264 -pix_fmt nv12 -f rawvideo
> > -vsync passthrough -vframes 45 -y out.yuv
> >
> > The root cause is the conflict between delay mechanism and -vframes.
> > FFmpeg won't output a frame if it's still receiving the initial packets,
> > so there is async between decode process and output. hwaccel_priv_data
> > in user thread won't be updated until the resolution changing
> > frame is output.
> >
> > As user context should reflect the state of the last frame that
> > was output to the user, hwaccel_priv_data should be updated
> > separately from decoding thread in time.
> >
> > Signed-off-by: Linjie Fu 
> > ---
> >  libavcodec/pthread_frame.c | 11 +--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
> > index 36ac0ac..cf7a575 100644
> > --- a/libavcodec/pthread_frame.c
> > +++ b/libavcodec/pthread_frame.c
> > @@ -282,7 +282,6 @@ static int
> > update_context_from_thread(AVCodecContext *dst, AVCodecContext
> *src,
> >  dst->sample_rate= src->sample_rate;
> >  dst->sample_fmt = src->sample_fmt;
> >  dst->channel_layout = src->channel_layout;
> > -dst->internal->hwaccel_priv_data = 
> > src->internal->hwaccel_priv_data;
> >
> >  if (!!dst->hw_frames_ctx != !!src->hw_frames_ctx ||
> >  (dst->hw_frames_ctx && dst->hw_frames_ctx->data != src-
> > >hw_frames_ctx->data)) {
> > @@ -410,6 +409,7 @@ static int submit_packet(PerThreadContext *p,
> > AVCodecContext *user_avctx,
> >  pthread_mutex_unlock(_thread->progress_mutex);
> >  }
> >
> > +p->avctx->internal->hwaccel_priv_data = prev_thread->avctx-
> > >internal->hwaccel_priv_data;
> >  err = update_context_from_thread(p->avctx, prev_thread->avctx, 0);
> >  if (err) {
> >  pthread_mutex_unlock(>mutex);
> > @@ -476,7 +476,7 @@ int ff_thread_decode_frame(AVCodecContext
> > *avctx,
> >  FrameThreadContext *fctx = avctx->internal->thread_ctx;
> >  int finished = fctx->next_finished;
> >  PerThreadContext *p;
> > -int err;
> > +int err, cur_decoding;
> >
> >  /* release the async lock, permitting blocked hwaccel threads to
> >   * go forward while we are in this function */
> > @@ -544,6 +544,13 @@ int ff_thread_decode_frame(AVCodecContext
> > *avctx,
> >
> >  if (fctx->next_decoding >= avctx->thread_count) fctx->next_decoding =
> 0;
> >
> > +/* update hwaccel_priv_data from decoding thread */
> > +cur_decoding = fctx->next_decoding - 1;
> > +if (cur_decoding < 0) cur_decoding += avctx->thread_count;
> > +
> > +p = >threads[cur_decoding];
> > +avctx->internal->hwaccel_priv_data = p->avctx->internal-
> > >hwaccel_priv_data;
> > +
> >  fctx->next_finished = finished;
> >
> >  /* return the size of the consumed packet if no error occurred */
> > --
> > 2.7.4
> 
> 
> Since previous concerns in https://patchwork.ffmpeg.org/patch/13723/
> could be  addressed in this version, ping for comments.
> 
A kind ping, comments are welcome.

-linjie
___
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/vp3: Check that theora is theora

2019-07-24 Thread Michael Niedermayer
Theora is forced to be non zero if it is zero and a sample
is asked for, as suggested by reimar

Fixes: Timeout (2min -> 600ms)
Fixes: 
15366/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_THEORA_fuzzer-5737849938247680

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer 
---
 libavcodec/vp3.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/libavcodec/vp3.c b/libavcodec/vp3.c
index a6f759ebf5..6ce901eda9 100644
--- a/libavcodec/vp3.c
+++ b/libavcodec/vp3.c
@@ -2957,6 +2957,10 @@ static int theora_decode_header(AVCodecContext *avctx, 
GetBitContext *gb)
 s->theora_header = 0;
 s->theora = get_bits_long(gb, 24);
 av_log(avctx, AV_LOG_DEBUG, "Theora bitstream version %X\n", s->theora);
+if (!s->theora) {
+s->theora = 1;
+avpriv_request_sample(s->avctx, "theora 0");
+}
 
 /* 3.2.0 aka alpha3 has the same frame orientation as original vp3
  * but previous versions have the image flipped relative to vp3 */
-- 
2.22.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, v3] fftools/ffmpeg_filter: add -autoscale to disable/enable the default scale

2019-07-24 Thread Fu, Linjie
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Paul B Mahol
> Sent: Wednesday, July 24, 2019 17:42
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH, v3] fftools/ffmpeg_filter: add -
> autoscale to disable/enable the default scale
> 
> On 7/24/19, Fu, Linjie  wrote:
> >> -Original Message-
> >> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> Behalf
> >> Of Gyan
> >> Sent: Wednesday, July 24, 2019 12:28
> >> To: ffmpeg-devel@ffmpeg.org
> >> Subject: Re: [FFmpeg-devel] [PATCH, v3] fftools/ffmpeg_filter: add -
> >> autoscale to disable/enable the default scale
> >>
> >>
> >>
> >> On 24-07-2019 08:15 AM, Fu, Linjie wrote:
> >> >> -Original Message-
> >> >> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> >> Behalf
> >> >> Of Gyan
> >> >> Sent: Wednesday, July 24, 2019 00:49
> >> >> To: ffmpeg-devel@ffmpeg.org
> >> >> Subject: Re: [FFmpeg-devel] [PATCH, v3] fftools/ffmpeg_filter: add -
> >> >> autoscale to disable/enable the default scale
> >> >>
> >> >>
> >> >>
> >> >> On 22-07-2019 01:40 PM, Fu, Linjie wrote:
> >>  -Original Message-
> >>  From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org]
> On
> >> >> Behalf
> >>  Of Gyan
> >>  Sent: Saturday, July 20, 2019 13:29
> >>  To: ffmpeg-devel@ffmpeg.org
> >>  Subject: Re: [FFmpeg-devel] [PATCH, v3] fftools/ffmpeg_filter: add
> -
> >>  autoscale to disable/enable the default scale
> >> > diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> >> > index cd35eb49c8..99121b6981 100644
> >> > --- a/doc/ffmpeg.texi
> >> > +++ b/doc/ffmpeg.texi
> >> > +@item -autoscale
> >> > +Automatically scale the video according to the resolution of first
> >> frame.
> >> > +Enabled by default, use @option{-noautoscale} to disable it.
> When
> >>  autoscale is
> >> > +disabled, all output frames might not be in the same resolution
> >> > and
> >> >> may
> >>  require
> >> > +some additional explicit processing according to your final
> >>  rendering/output
> >> > +destination. Disabling autoscale may not work in all situations.
> >> >> Therefore,
> >>  it
> >> > +is not recommended to disable it unless you really know what
> you
> >> are
> >>  doing.
> >> > +Disable autoscale at your own risk.
> >>  Since the auto scaling happens at the end of the graph, what may
> the
> >>  "additional explicit processing" be?
> >> >>> Vpp processing may not be influenced,  a warning in transcode is
> >> >>> needed.
> >> >>> The expression seems to be improper, how about:
> >> >>>
> >> >>> "When autoscale is disabled, all output frames of filter graph might
> >> >>> not
> >> be
> >> >> in the same
> >> >>> resolution and may be inadequate for encoder/muxer."
> >> >>>
> >> >>> or other suggestions?
> >> >>>
> >> > @@ -3640,6 +3642,12 @@ const OptionDef options[] = {
> >> > { "autorotate",   HAS_ARG | OPT_BOOL | OPT_SPEC |
> >> >   OPT_EXPERT | OPT_INPUT,
> >> > { .off =
> >>  OFFSET(autorotate) },
> >> > "automatically insert correct rotate filters" },
> >> > +{ "autoscale",HAS_ARG | OPT_BOOL | OPT_SPEC |
> >> > +  OPT_EXPERT | OPT_INPUT,
> >> >  { .off =
> >>  OFFSET(autoscale) },
> >> > +"automatically insert a scale filter at the end of the
> >> > filter graph if
> >> a
> >>  resolution"
> >> > +"change is detected. This ensures all frames are the same
> >> >> resolution
> >>  as the first frame"
> >> > +"when they leave the filter chain (this option is enabled
> >> > by
> >> >> default)."
> >> > +"If disabled, some encoders/muxers may not support this
> >> mode."},
> >>  Which muxers can detect or check for prop changes within coded
> >>  bitstreams? Which encoders are known to be able to handle
> >>  changing resolution?
> >> >>> It's not supported currently (even in libvpx-vp9, since vp9 supports
> >> >> dynamic resolution in spec).
> >> >>> I don't intend to be so absolutely in doc,  will it be better for you
> >> >>> to
> >> modify
> >> >> like:
> >> >>> "If disabled, encoders/muxers won't support this mode currently."
> >> >> So other than`-c:v rawvideo -f rawvideo`, there is no combination
> >> >> that supports changing frame sizes?
> >> > I didn't check all encoders.
> >> > But since the resolution changing is detectable, some additional reinit
> >> procedures
> >> > can be added to change resolution at IDR frame and make the encoder
> >> works.
> >> >
> >> They could, but as of now, which encoder + muxer combinations are
> >> **known** to work?
> >>
> > As far as I tried, no.
> > (pipeline failed in libx264,  garbage exists in libx265 and libvpx-vp9 )
> 
> Than how this change can 

Re: [FFmpeg-devel] [PATCH] avcodec/rl2: set dimensions

2019-07-24 Thread Lynne
Jul 24, 2019, 11:08 AM by mich...@niedermayer.cc:

>
> What did you expect ? IIRC you have asked for whole classes of security
> issues to be not fixed.
>
> Something like that would require a vote and majority of developers.
>

The way I interpret this: "Of course I ignored you, you're mental!", doesn't 
help. And I don't think its just me.
And you do remember incorrectly in saying that I want this whole class of 
security issues not fixed. In this thread I specifically raised the issue of 
what is considered to be a security issue by asking whether a speedup of 
failing to decode from 2 to 0.4 seconds is considered such or what's considered 
acceptable in general.
And I think I'll disagree that it is. 16 seconds to 2 seconds I can accept, but 
not 2 to 0.4.



>> These patches affect decoding of real world broken files in favor of fixing 
>> specially crafted fuzzed files.
>>
>
> Iam happy to look into such cases. Can you provide me with such
> "real world broken files"?
> Its not intended to worsen the output from such files
>

Simple logical analysis, "if file is somewhat broken, don't try decoding" does 
very well indicate that it won't only apply for _this_ broken file, but in 
general.
Thus, this is for you to prove. I've said it before that otherwise its a burden 
to other developers to have to screen all of these patches.



>> Sure, protecting against ddos attacts is important, but not important enough 
>> to make decoders give up early and return nothing. Especially in cases where 
>> the timeout speedup is of the order of 2s to 400ms.
>> Yet in all of those timeout patches all you've cared about is shutting up 
>> the tool. You've never once shown any figures if this could affect decoding, 
>> because its a lot harder than just showing they fix something some tool 
>> calls a timeout and forgetting about it. You haven't even commented on this 
>> when I asked you on IRC.
>> You also sneak this type of patches in when there's an overflow later on 
>> during decoding, which is completely incorrect in almost all cases, for the 
>> same reason above.
>>
>
> if you know of issues in a patch or commit you should report this
> during patch review or as soon as you find out about the issue
> as a reply to that patch or commit or as mail to the author.
>

That's what I'm doing.
That aside, you've completely ignored my statements on what's considered 
acceptable, showing figures, and sneaking this type of patches to fix undefined 
behavior.
Making your reply a simple refutation, rather than addressing anything I've 
said.
So I'm asking you again, what is considered a security issue and what is 
considered acceptable? And what is considered not a security issue but a 
complaint from an overzealous automated tool.



> So the author or any other developer can fix it or the patch is on hold
> until its fixed.
>

Is not commiting it at all because there is no issue, adds unnecessary code 
which makes things worse, only to speed failing to decode by milliseconds for a 
very specific crafted input to silence an automated tool completely not an 
option?



> Complaining out of context about unspecified issues. Or well IRC  (i dont
> know what you refer to here at all either)
> Is just guranteed to lead to frustration because that wont and cant lead to
> issues being fixed
>

I am complaining in the context of this thread about specific issues that I'm 
worried all of these patches have, and I'm not alone, despite you trying to 
label someone else's concerned reply as a personal attack.
I'm sure other developers would agree if asked, just probably not like this in 
a situation where a reply is labelled as an attack.



> PS: Again, i strongly suggest we work together to find acceptable solutions
> to these issues. Because that would lead to good solutions, and everyone
> being non frustrated. When one demands "dont fix it" that is what leads
> to a fight and frustration and a blocked patch so really both sides are
> unhappy and frustrated
>

We are. Or I'm trying to, but being simply refuted only goes so far.

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/vp3: Check that theora is theora

2019-07-24 Thread Michael Niedermayer
On Mon, Jul 22, 2019 at 07:07:57AM +0200, Reimar Döffinger wrote:
> 
> 
> On 22.07.2019, at 01:25, Michael Niedermayer  wrote:
> 
> > Fixes: Timeout (2min -> 100ms)
> > Fixes: 
> > 15366/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_THEORA_fuzzer-5737849938247680
> > 
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> > ---
> > libavcodec/vp3.c | 2 ++
> > 1 file changed, 2 insertions(+)
> > 
> > diff --git a/libavcodec/vp3.c b/libavcodec/vp3.c
> > index a6f759ebf5..8a165c1275 100644
> > --- a/libavcodec/vp3.c
> > +++ b/libavcodec/vp3.c
> > @@ -2957,6 +2957,8 @@ static int theora_decode_header(AVCodecContext 
> > *avctx, GetBitContext *gb)
> > s->theora_header = 0;
> > s->theora = get_bits_long(gb, 24);
> > av_log(avctx, AV_LOG_DEBUG, "Theora bitstream version %X\n", s->theora);
> > +if (!s->theora)
> > +return AVERROR_INVALIDDATA;
> 
> That seems rather strict, a 1-bit error in this field and we don't even try?
> Maybe set to 1 instead with a request for sample?

ok, will post a new patch

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

Elect your leaders based on what they did after the last election, not
based on what they say before an election.



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] avfilter/af_dynaudnorm: add more descriptive aliases for options

2019-07-24 Thread Paul B Mahol
Hi,

patch attached.


0001-avfilter-af_dynaudnorm-add-more-descriptive-aliases-.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".

Re: [FFmpeg-devel] [PATCH] avfilter: add square audio source filter

2019-07-24 Thread Paul B Mahol
On 7/14/19, Paul B Mahol  wrote:
> Signed-off-by: Paul B Mahol 
> ---
>  doc/filters.texi | 44 
>  libavfilter/Makefile |  1 +
>  libavfilter/allfilters.c |  1 +
>  libavfilter/asrc_sine.c  | 86 ++--
>  4 files changed, 119 insertions(+), 13 deletions(-)
>

Will apply ASAP!
___
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 v5 1/2] lavc/mjpegdec: Decode Huffman-coded lossless JPEGs embedded in DNGs

2019-07-24 Thread velocityra
From: Nick Renieris 

Main image data in DNGs is usually comprised of tiles, each of which is a 
Huffman-encoded lossless JPEG.

Tested for ljpeg regressions with:
`ffmpeg -f lavfi -i testsrc=d=1 -vcodec ljpeg test.avi`
`ffmpeg test.avi out.avi`
The modified code in ljpeg_decode_rgb_scan runs without issues.

Signed-off-by: Nick Renieris 
---
 libavcodec/mjpegdec.c | 54 ---
 libavcodec/mjpegdec.h |  1 +
 2 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
index a65bc8df15..225833d1bd 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -412,6 +412,14 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s)
 return AVERROR_PATCHWELCOME;
 }
 
+/* Lossless JPEGs encoded in DNGs are commonly bayer-encoded. They contain 
2
+   interleaved components and the width stored in their SOF3 markers is the
+   width of each one.  We only output a single component, therefore we need
+   to adjust the output image width. */
+if (s->lossless == 1 && nb_components == 2) {
+s->bayer = 1;
+width *= 2;
+}
 
 /* if different size, realloc/alloc picture */
 if (width != s->width || height != s->height || bits != s->bits ||
@@ -488,6 +496,9 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s)
 }
 
 switch (pix_fmt_id) {
+case 0x: /* for bayer-encoded huffman lossless JPEGs embedded 
in DNGs */
+s->avctx->pix_fmt = AV_PIX_FMT_GRAY16LE;
+break;
 case 0x1100:
 if (s->rgb)
 s->avctx->pix_fmt = s->bits <= 9 ? AV_PIX_FMT_BGR24 : 
AV_PIX_FMT_BGR48;
@@ -1041,26 +1052,36 @@ static int handle_rstn(MJpegDecodeContext *s, int 
nb_components)
 return reset;
 }
 
+/* Handles 1 to 4 components */
 static int ljpeg_decode_rgb_scan(MJpegDecodeContext *s, int nb_components, int 
predictor, int point_transform)
 {
 int i, mb_x, mb_y;
+unsigned width;
 uint16_t (*buffer)[4];
 int left[4], top[4], topleft[4];
 const int linesize = s->linesize[0];
 const int mask = ((1 << s->bits) - 1) << point_transform;
 int resync_mb_y = 0;
 int resync_mb_x = 0;
+int vpred[6];
 
-if (s->nb_components != 3 && s->nb_components != 4)
+if (s->nb_components <= 0 || s->nb_components > 4)
 return AVERROR_INVALIDDATA;
+
 if (s->v_max != 1 || s->h_max != 1 || !s->lossless)
 return AVERROR_INVALIDDATA;
 
-
 s->restart_count = s->restart_interval;
 
-av_fast_malloc(>ljpeg_buffer, >ljpeg_buffer_size,
-   (unsigned)s->mb_width * 4 * sizeof(s->ljpeg_buffer[0][0]));
+if (s->restart_interval == 0)
+s->restart_interval = INT_MAX;
+
+if (s->bayer)
+width = s->mb_width / nb_components; /* Interleaved, width stored is 
the total so need to divide */
+else
+width = s->mb_width;
+
+av_fast_malloc(>ljpeg_buffer, >ljpeg_buffer_size, width * 4 * 
sizeof(s->ljpeg_buffer[0][0]));
 if (!s->ljpeg_buffer)
 return AVERROR(ENOMEM);
 
@@ -1078,7 +1099,12 @@ static int ljpeg_decode_rgb_scan(MJpegDecodeContext *s, 
int nb_components, int p
 for (i = 0; i < 4; i++)
 top[i] = left[i] = topleft[i] = buffer[0][i];
 
-for (mb_x = 0; mb_x < s->mb_width; mb_x++) {
+if (mb_y * s->width % s->restart_interval == 0) {
+for (i = 0; i < 6; i++)
+vpred[i] = 1 << (s->bits-1);
+}
+
+for (mb_x = 0; mb_x < width; mb_x++) {
 int modified_predictor = predictor;
 
 if (get_bits_left(>gb) < 1) {
@@ -1102,12 +1128,19 @@ static int ljpeg_decode_rgb_scan(MJpegDecodeContext *s, 
int nb_components, int p
 topleft[i] = top[i];
 top[i] = buffer[mb_x][i];
 
-PREDICT(pred, topleft[i], top[i], left[i], modified_predictor);
-
 dc = mjpeg_decode_dc(s, s->dc_index[i]);
 if(dc == 0xF)
 return -1;
 
+if (mb_x) {
+pred = left[i];
+} else {
+vpred[i] += dc;
+pred = vpred[i] - dc;
+}
+
+PREDICT(pred, topleft[i], top[i], pred, modified_predictor);
+
 left[i] = buffer[mb_x][i] =
 mask & (pred + (unsigned)(dc * (1 << point_transform)));
 }
@@ -1151,6 +1184,11 @@ static int ljpeg_decode_rgb_scan(MJpegDecodeContext *s, 
int nb_components, int p
 ptr[3*mb_x + 0] = buffer[mb_x][1] + ptr[3*mb_x + 1];
 ptr[3*mb_x + 2] = buffer[mb_x][2] + ptr[3*mb_x + 1];
 }
+} else if (s->bayer && nb_components == 2) {
+for (mb_x = 0; mb_x < width; mb_x++) {
+((uint16_t*)ptr)[2*mb_x + 0] = buffer[mb_x][0];
+((uint16_t*)ptr)[2*mb_x + 1] = buffer[mb_x][1];
+}
 } else {

[FFmpeg-devel] [PATCH v5 2/2] lavc/tiff: Decode embedded JPEGs in DNG images

2019-07-24 Thread velocityra
From: Nick Renieris 

Used a technique similar to lavc/tdsc.c for invoking the MJPEG decoder.

This commit adds support for:
- DNG tiles
- DNG tile huffman lossless JPEG decoding
- DNG 8-bpp ("packed" as dcraw calls it) decoding
- DNG color scaling [1]
  - LinearizationTable tag
  - BlackLevel tag

[1]: As specified in the DNG Specification - Chapter 5

Signed-off-by: Nick Renieris 
---
 configure   |   1 +
 libavcodec/Makefile |   2 +-
 libavcodec/tiff.c   | 310 +++-
 libavcodec/tiff.h   |   2 +
 4 files changed, 307 insertions(+), 8 deletions(-)

diff --git a/configure b/configure
index 5a4f507246..54163822a8 100755
--- a/configure
+++ b/configure
@@ -2813,6 +2813,7 @@ theora_decoder_select="vp3_decoder"
 thp_decoder_select="mjpeg_decoder"
 tiff_decoder_suggest="zlib lzma"
 tiff_encoder_suggest="zlib"
+tiff_decoder_select="mjpeg_decoder"
 truehd_decoder_select="mlp_parser"
 truehd_encoder_select="lpc audio_frame_queue"
 truemotion2_decoder_select="bswapdsp"
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 3cd73fbcc6..f814c69996 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -616,7 +616,7 @@ OBJS-$(CONFIG_TARGA_ENCODER)   += targaenc.o rle.o
 OBJS-$(CONFIG_TARGA_Y216_DECODER)  += targa_y216dec.o
 OBJS-$(CONFIG_TDSC_DECODER)+= tdsc.o
 OBJS-$(CONFIG_TIERTEXSEQVIDEO_DECODER) += tiertexseqv.o
-OBJS-$(CONFIG_TIFF_DECODER)+= tiff.o lzw.o faxcompr.o tiff_data.o 
tiff_common.o
+OBJS-$(CONFIG_TIFF_DECODER)+= tiff.o lzw.o faxcompr.o tiff_data.o 
tiff_common.o mjpegdec.o
 OBJS-$(CONFIG_TIFF_ENCODER)+= tiffenc.o rle.o lzwenc.o tiff_data.o
 OBJS-$(CONFIG_TMV_DECODER) += tmv.o cga_data.o
 OBJS-$(CONFIG_TRUEHD_DECODER)  += mlpdec.o mlpdsp.o
diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c
index c520d7df83..4f6ba256c6 100644
--- a/libavcodec/tiff.c
+++ b/libavcodec/tiff.c
@@ -46,6 +46,7 @@
 #include "mathops.h"
 #include "tiff.h"
 #include "tiff_data.h"
+#include "mjpegdec.h"
 #include "thread.h"
 #include "get_bits.h"
 
@@ -54,6 +55,10 @@ typedef struct TiffContext {
 AVCodecContext *avctx;
 GetByteContext gb;
 
+/* JPEG decoding for DNG */
+AVCodecContext *avctx_mjpeg; // wrapper context for MJPEG
+AVFrame *jpgframe;   // decoded JPEG tile
+
 int get_subimage;
 uint16_t get_page;
 int get_thumbnail;
@@ -76,7 +81,9 @@ typedef struct TiffContext {
 
 int is_bayer;
 uint8_t pattern[4];
+unsigned black_level;
 unsigned white_level;
+const uint16_t *dng_lut; // Pointer to DNG linearization table
 
 uint32_t sub_ifd;
 uint16_t cur_page;
@@ -86,6 +93,14 @@ typedef struct TiffContext {
 int stripsizesoff, stripsize, stripoff, strippos;
 LZWState *lzw;
 
+/* Tile support */
+int is_tiled;
+int tile_byte_counts_offset, tile_offsets_offset;
+int tile_width, tile_length;
+int tile_count;
+
+int is_jpeg; // 0 - Not JPEG, 1 - JPEG, 2 - New JPEG
+
 uint8_t *deinvert_buf;
 int deinvert_buf_size;
 uint8_t *yuv_line;
@@ -257,6 +272,9 @@ static int add_metadata(int count, int type,
 };
 }
 
+static void av_always_inline dng_blit(TiffContext *s, uint8_t *dst, int 
dst_stride,
+  const uint8_t *src, int src_stride, int 
width, int height, int is_u16);
+
 static void av_always_inline horizontal_fill(TiffContext *s,
  unsigned int bpp, uint8_t* dst,
  int usePtr, const uint8_t *src,
@@ -712,6 +730,203 @@ static int tiff_unpack_strip(TiffContext *s, AVFrame *p, 
uint8_t *dst, int strid
 return 0;
 }
 
+/**
+ * Map stored raw sensor values into linear reference values.
+ * See: DNG Specification - Chapter 5
+ */
+static uint16_t av_always_inline dng_raw_to_linear16(uint16_t value,
+const uint16_t *lut,
+uint16_t black_level,
+float scale_factor) {
+// Lookup table lookup
+if (lut) value = lut[value];
+
+// Black level subtraction
+value = av_clip_uint16_c((unsigned)value - black_level);
+
+// Color scaling
+value = av_clip_uint16_c((unsigned)(((float)value * scale_factor) * 
0x));
+
+return value;
+}
+
+static uint16_t av_always_inline dng_raw_to_linear8(uint16_t value,
+const uint16_t *lut,
+uint16_t black_level,
+float scale_factor) {
+return dng_raw_to_linear16(value, lut, black_level, scale_factor) >> 8;
+}
+
+static void dng_blit(TiffContext *s, uint8_t *dst, int dst_stride,
+ const uint8_t *src, int src_stride,
+ int width, int height, int is_u16)
+{
+int 

Re: [FFmpeg-devel] [PATCH] lavc/tiff: Decode embedded JPEGs in DNG images

2019-07-24 Thread Nick Renieris
The latest patch is unrelated to the gamma stuff (which in hindsight
isn't accurately implemented, so never mind), it has a regression fix
for z8pf.dng and adds compatibility to the following:
https://www.camerafv5.com/pages/raw-samples.php
https://www.camerafv5.com/pages/nexus6-raw-samples.php
https://www.chris-juettner.de/projekte/raw-samples-comparison-download/dji-mavic-air
etc

>Do you plan to implement camera native RGB to CIE XYZ mapping as specified
in DNG Spec 1.4.0.0 Chapter 6 ?

Perhaps. I haven't looked at it much but it may be too much work for
the time I have left, I will talk with my mentor on whether to
prioritize it (a lot of images look fine without it).

Στις Τετ, 24 Ιουλ 2019 στις 1:06 μ.μ., ο/η  έγραψε:
>
> From: Nick Renieris 
>
> Used a technique similar to lavc/tdsc.c for invoking the MJPEG decoder.
>
> This commit adds support for:
> - DNG tiles
> - DNG tile huffman lossless JPEG decoding
> - DNG color scaling [1]
>   - LinearizationTable tag
>   - BlackLevel tag
>
> [1]: As specified in the DNG Specification - Chapter 5
>
> Signed-off-by: Nick Renieris 
> ---
>  libavcodec/tiff.c | 16 
>  1 file changed, 16 deletions(-)
>
> diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c
> index 423eaf0647..4f6ba256c6 100644
> --- a/libavcodec/tiff.c
> +++ b/libavcodec/tiff.c
> @@ -280,19 +280,6 @@ static void av_always_inline horizontal_fill(TiffContext 
> *s,
>   int usePtr, const uint8_t *src,
>   uint8_t c, int width, int 
> offset)
>  {
> -/* Handle DNG images with uncompressed strips (non-tiled) */
> -if (s->tiff_type == TIFF_TYPE_DNG || s->tiff_type == 
> TIFF_TYPE_CINEMADNG) {
> -dng_blit(s,
> - dst + offset,
> - width,
> - src,
> - width,
> - width,
> - 1,
> - 0);
> -return;
> -}
> -
>  switch (bpp) {
>  case 1:
>  while (--width >= 0) {
> @@ -1799,9 +1786,6 @@ again:
>  } else if (s->is_tiled) {
>  av_log(avctx, AV_LOG_ERROR, "DNG uncompressed tiled images are 
> not supported\n");
>  return AVERROR_PATCHWELCOME;
> -} else if (s->bpp != 8) {
> -av_log(avctx, AV_LOG_ERROR, "DNG uncompressed non-tiled non-8bpp 
> images are not supported\n");
> -return AVERROR_PATCHWELCOME;
>  }
>  }
>
> --
> 2.21.0.windows.1
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec/rl2: set dimensions

2019-07-24 Thread Michael Niedermayer
On Tue, Jul 23, 2019 at 04:42:32AM +0200, Lynne wrote:
> Jul 23, 2019, 12:00 AM by mich...@niedermayer.cc:
> 
> > On Tue, Jul 23, 2019 at 12:13:51AM +0200, Lynne wrote:
> >
> >> Jul 22, 2019, 10:57 PM by mich...@niedermayer.cc:
> >>
> >> > The dimensions are always 320x200 they are hardcoded in the demuxer.
> >> > Hardcode them instead in the decoder.
> >> >
> >> > Fixes: Timeout (16sec -> 400ms)
> >> > Fixes: 
> >> > 15574/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_RL2_fuzzer-5158614072819712
> >> >
> >> > Found-by: continuous fuzzing process 
> >> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> >> > Signed-off-by: Michael Niedermayer 
> >> > ---
> >> >  libavcodec/rl2.c | 5 +
> >> >  1 file changed, 5 insertions(+)
> >> >
> >> > diff --git a/libavcodec/rl2.c b/libavcodec/rl2.c
> >> > index 6662979c52..2d336a61e5 100644
> >> > --- a/libavcodec/rl2.c
> >> > +++ b/libavcodec/rl2.c
> >> > @@ -134,10 +134,15 @@ static av_cold int rl2_decode_init(AVCodecContext 
> >> > *avctx)
> >> >  Rl2Context *s = avctx->priv_data;
> >> >  int back_size;
> >> >  int i;
> >> > +int ret;
> >> > 
> >> >  s->avctx   = avctx;
> >> >  avctx->pix_fmt = AV_PIX_FMT_PAL8;
> >> > 
> >> > +ret = ff_set_dimensions(avctx, 320, 200);
> >> > +if (ret < 0)
> >> > +return ret;
> >> > +
> >> >  /** parse extra data */
> >> >  if (!avctx->extradata || avctx->extradata_size < EXTRADATA1_SIZE) {
> >> >  av_log(avctx, AV_LOG_ERROR, "invalid extradata size\n");
> >> > -- 
> >> > 2.22.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".
> >> >
> >>
> >> If the size somehow gets lost between lavf and lavc then the problem is 
> >> there, and should be solved by not fudging the decoder.
> >>
> >
> > libavcodec is used by many applications which do not use libavformat.
> > for all i know and what is documented in the only reference spec i have. 
> > the resolution is a constant its always 320x200 for this codec. So its
> > natural to have the decoder know about its own resolution. 
> >
> 
> Nonetheless, while I agree with having both decoder and demuxer hardcode the 
> resolution, I still think this is fudging.
> 
> 
> 
> >> Moreover, if neither the codec, nor container can change the resolution, 
> >> then how does that make anything different?
> >>
> >
> >
> >> Please, stop it with those patches. It takes energy and some worry to have 
> >> to open up the ML and scan it for bad patches from people who really 
> >> should know better every time.
> >>
> >
> > Please stop with these attacks, this is just making people become
> > mutually more aggressive and filled with hatred.
> >
> 

> Of course I'm frustrated, I've been ignored.

What did you expect ? IIRC you have asked for whole classes of security
issues to be not fixed.

Something like that would require a vote and majority of developers.



> These patches affect decoding of real world broken files in favor of fixing 
> specially crafted fuzzed files. 

Iam happy to look into such cases. Can you provide me with such
"real world broken files"?
Its not intended to worsen the output from such files


> Sure, protecting against ddos attacts is important, but not important enough 
> to make decoders give up early and return nothing. Especially in cases where 
> the timeout speedup is of the order of 2s to 400ms.
> Yet in all of those timeout patches all you've cared about is shutting up the 
> tool. You've never once shown any figures if this could affect decoding, 
> because its a lot harder than just showing they fix something some tool calls 
> a timeout and forgetting about it. You haven't even commented on this when I 
> asked you on IRC.
> You also sneak this type of patches in when there's an overflow later on 
> during decoding, which is completely incorrect in almost all cases, for the 
> same reason above.

if you know of issues in a patch or commit you should report this
during patch review or as soon as you find out about the issue
as a reply to that patch or commit or as mail to the author.

So the author or any other developer can fix it or the patch is on hold
until its fixed.

Complaining out of context about unspecified issues. Or well IRC  (i dont
know what you refer to here at all either)
Is just guranteed to lead to frustration because that wont and cant lead to
issues being fixed

PS: Again, i strongly suggest we work together to find acceptable solutions
to these issues. Because that would lead to good solutions, and everyone
being non frustrated. When one demands "dont fix it" that is what leads
to a fight and frustration and a blocked patch so really both sides are
unhappy and frustrated


Thanks

[...]

-- 
Michael 

[FFmpeg-devel] [PATCH] lavc/tiff: Decode embedded JPEGs in DNG images

2019-07-24 Thread velocityra
From: Nick Renieris 

Used a technique similar to lavc/tdsc.c for invoking the MJPEG decoder.

This commit adds support for:
- DNG tiles
- DNG tile huffman lossless JPEG decoding
- DNG color scaling [1]
  - LinearizationTable tag
  - BlackLevel tag

[1]: As specified in the DNG Specification - Chapter 5

Signed-off-by: Nick Renieris 
---
 libavcodec/tiff.c | 16 
 1 file changed, 16 deletions(-)

diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c
index 423eaf0647..4f6ba256c6 100644
--- a/libavcodec/tiff.c
+++ b/libavcodec/tiff.c
@@ -280,19 +280,6 @@ static void av_always_inline horizontal_fill(TiffContext 
*s,
  int usePtr, const uint8_t *src,
  uint8_t c, int width, int offset)
 {
-/* Handle DNG images with uncompressed strips (non-tiled) */
-if (s->tiff_type == TIFF_TYPE_DNG || s->tiff_type == TIFF_TYPE_CINEMADNG) {
-dng_blit(s,
- dst + offset,
- width,
- src,
- width,
- width,
- 1,
- 0);
-return;
-}
-
 switch (bpp) {
 case 1:
 while (--width >= 0) {
@@ -1799,9 +1786,6 @@ again:
 } else if (s->is_tiled) {
 av_log(avctx, AV_LOG_ERROR, "DNG uncompressed tiled images are not 
supported\n");
 return AVERROR_PATCHWELCOME;
-} else if (s->bpp != 8) {
-av_log(avctx, AV_LOG_ERROR, "DNG uncompressed non-tiled non-8bpp 
images are not supported\n");
-return AVERROR_PATCHWELCOME;
 }
 }
 
-- 
2.21.0.windows.1

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3 2/2] lavc/tiff: Decode embedded JPEGs in DNG images

2019-07-24 Thread Remi Achard
> Colors are wrong and I'm pretty sure I know why, that's TODO.
>

Do you plan to implement camera native RGB to CIE XYZ mapping as specified
in DNG Spec 1.4.0.0 Chapter 6 ?
___
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] fftools/ffmpeg_filter: add -autoscale to disable/enable the default scale

2019-07-24 Thread Paul B Mahol
On 7/24/19, Fu, Linjie  wrote:
>> -Original Message-
>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
>> Of Gyan
>> Sent: Wednesday, July 24, 2019 12:28
>> To: ffmpeg-devel@ffmpeg.org
>> Subject: Re: [FFmpeg-devel] [PATCH, v3] fftools/ffmpeg_filter: add -
>> autoscale to disable/enable the default scale
>>
>>
>>
>> On 24-07-2019 08:15 AM, Fu, Linjie wrote:
>> >> -Original Message-
>> >> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
>> Behalf
>> >> Of Gyan
>> >> Sent: Wednesday, July 24, 2019 00:49
>> >> To: ffmpeg-devel@ffmpeg.org
>> >> Subject: Re: [FFmpeg-devel] [PATCH, v3] fftools/ffmpeg_filter: add -
>> >> autoscale to disable/enable the default scale
>> >>
>> >>
>> >>
>> >> On 22-07-2019 01:40 PM, Fu, Linjie wrote:
>>  -Original Message-
>>  From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
>> >> Behalf
>>  Of Gyan
>>  Sent: Saturday, July 20, 2019 13:29
>>  To: ffmpeg-devel@ffmpeg.org
>>  Subject: Re: [FFmpeg-devel] [PATCH, v3] fftools/ffmpeg_filter: add -
>>  autoscale to disable/enable the default scale
>> > diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
>> > index cd35eb49c8..99121b6981 100644
>> > --- a/doc/ffmpeg.texi
>> > +++ b/doc/ffmpeg.texi
>> > +@item -autoscale
>> > +Automatically scale the video according to the resolution of first
>> frame.
>> > +Enabled by default, use @option{-noautoscale} to disable it. When
>>  autoscale is
>> > +disabled, all output frames might not be in the same resolution
>> > and
>> >> may
>>  require
>> > +some additional explicit processing according to your final
>>  rendering/output
>> > +destination. Disabling autoscale may not work in all situations.
>> >> Therefore,
>>  it
>> > +is not recommended to disable it unless you really know what you
>> are
>>  doing.
>> > +Disable autoscale at your own risk.
>>  Since the auto scaling happens at the end of the graph, what may the
>>  "additional explicit processing" be?
>> >>> Vpp processing may not be influenced,  a warning in transcode is
>> >>> needed.
>> >>> The expression seems to be improper, how about:
>> >>>
>> >>> "When autoscale is disabled, all output frames of filter graph might
>> >>> not
>> be
>> >> in the same
>> >>> resolution and may be inadequate for encoder/muxer."
>> >>>
>> >>> or other suggestions?
>> >>>
>> > @@ -3640,6 +3642,12 @@ const OptionDef options[] = {
>> > { "autorotate",   HAS_ARG | OPT_BOOL | OPT_SPEC |
>> >   OPT_EXPERT | OPT_INPUT,
>> > { .off =
>>  OFFSET(autorotate) },
>> > "automatically insert correct rotate filters" },
>> > +{ "autoscale",HAS_ARG | OPT_BOOL | OPT_SPEC |
>> > +  OPT_EXPERT | OPT_INPUT,
>> >  { .off =
>>  OFFSET(autoscale) },
>> > +"automatically insert a scale filter at the end of the
>> > filter graph if
>> a
>>  resolution"
>> > +"change is detected. This ensures all frames are the same
>> >> resolution
>>  as the first frame"
>> > +"when they leave the filter chain (this option is enabled
>> > by
>> >> default)."
>> > +"If disabled, some encoders/muxers may not support this
>> mode."},
>>  Which muxers can detect or check for prop changes within coded
>>  bitstreams? Which encoders are known to be able to handle
>>  changing resolution?
>> >>> It's not supported currently (even in libvpx-vp9, since vp9 supports
>> >> dynamic resolution in spec).
>> >>> I don't intend to be so absolutely in doc,  will it be better for you
>> >>> to
>> modify
>> >> like:
>> >>> "If disabled, encoders/muxers won't support this mode currently."
>> >> So other than`-c:v rawvideo -f rawvideo`, there is no combination
>> >> that supports changing frame sizes?
>> > I didn't check all encoders.
>> > But since the resolution changing is detectable, some additional reinit
>> procedures
>> > can be added to change resolution at IDR frame and make the encoder
>> works.
>> >
>> They could, but as of now, which encoder + muxer combinations are
>> **known** to work?
>>
> As far as I tried, no.
> (pipeline failed in libx264,  garbage exists in libx265 and libvpx-vp9 )

Than how this change can be useful to us?

It will just increase maintenance burden on other developers.
___
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/adxenc: add EOF header

2019-07-24 Thread Paul B Mahol
Hi,

patch attached.


0001-avcodec-adxenc-add-EOF-header.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".

Re: [FFmpeg-devel] [PATCH, v3] fftools/ffmpeg_filter: add -autoscale to disable/enable the default scale

2019-07-24 Thread Fu, Linjie
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Gyan
> Sent: Wednesday, July 24, 2019 12:28
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH, v3] fftools/ffmpeg_filter: add -
> autoscale to disable/enable the default scale
> 
> 
> 
> On 24-07-2019 08:15 AM, Fu, Linjie wrote:
> >> -Original Message-
> >> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> Behalf
> >> Of Gyan
> >> Sent: Wednesday, July 24, 2019 00:49
> >> To: ffmpeg-devel@ffmpeg.org
> >> Subject: Re: [FFmpeg-devel] [PATCH, v3] fftools/ffmpeg_filter: add -
> >> autoscale to disable/enable the default scale
> >>
> >>
> >>
> >> On 22-07-2019 01:40 PM, Fu, Linjie wrote:
>  -Original Message-
>  From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> >> Behalf
>  Of Gyan
>  Sent: Saturday, July 20, 2019 13:29
>  To: ffmpeg-devel@ffmpeg.org
>  Subject: Re: [FFmpeg-devel] [PATCH, v3] fftools/ffmpeg_filter: add -
>  autoscale to disable/enable the default scale
> > diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> > index cd35eb49c8..99121b6981 100644
> > --- a/doc/ffmpeg.texi
> > +++ b/doc/ffmpeg.texi
> > +@item -autoscale
> > +Automatically scale the video according to the resolution of first
> frame.
> > +Enabled by default, use @option{-noautoscale} to disable it. When
>  autoscale is
> > +disabled, all output frames might not be in the same resolution and
> >> may
>  require
> > +some additional explicit processing according to your final
>  rendering/output
> > +destination. Disabling autoscale may not work in all situations.
> >> Therefore,
>  it
> > +is not recommended to disable it unless you really know what you
> are
>  doing.
> > +Disable autoscale at your own risk.
>  Since the auto scaling happens at the end of the graph, what may the
>  "additional explicit processing" be?
> >>> Vpp processing may not be influenced,  a warning in transcode is needed.
> >>> The expression seems to be improper, how about:
> >>>
> >>> "When autoscale is disabled, all output frames of filter graph might not
> be
> >> in the same
> >>> resolution and may be inadequate for encoder/muxer."
> >>>
> >>> or other suggestions?
> >>>
> > @@ -3640,6 +3642,12 @@ const OptionDef options[] = {
> > { "autorotate",   HAS_ARG | OPT_BOOL | OPT_SPEC |
> >   OPT_EXPERT | OPT_INPUT,   
> >  { .off =
>  OFFSET(autorotate) },
> > "automatically insert correct rotate filters" },
> > +{ "autoscale",HAS_ARG | OPT_BOOL | OPT_SPEC |
> > +  OPT_EXPERT | OPT_INPUT,  
> >   { .off =
>  OFFSET(autoscale) },
> > +"automatically insert a scale filter at the end of the filter 
> > graph if
> a
>  resolution"
> > +"change is detected. This ensures all frames are the same
> >> resolution
>  as the first frame"
> > +"when they leave the filter chain (this option is enabled by
> >> default)."
> > +"If disabled, some encoders/muxers may not support this
> mode."},
>  Which muxers can detect or check for prop changes within coded
>  bitstreams? Which encoders are known to be able to handle
>  changing resolution?
> >>> It's not supported currently (even in libvpx-vp9, since vp9 supports
> >> dynamic resolution in spec).
> >>> I don't intend to be so absolutely in doc,  will it be better for you to
> modify
> >> like:
> >>> "If disabled, encoders/muxers won't support this mode currently."
> >> So other than    `-c:v rawvideo -f rawvideo`, there is no combination
> >> that supports changing frame sizes?
> > I didn't check all encoders.
> > But since the resolution changing is detectable, some additional reinit
> procedures
> > can be added to change resolution at IDR frame and make the encoder
> works.
> >
> They could, but as of now, which encoder + muxer combinations are
> **known** to work?
> 
As far as I tried, no.
(pipeline failed in libx264,  garbage exists in libx265 and libvpx-vp9 )
___
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 2/2] lavc/tiff: Decode embedded JPEGs in DNG images

2019-07-24 Thread Nick Renieris
Some example DNGs before/after the gamma correction:
https://i.imgur.com/m3Qgrwb.jpg
https://i.imgur.com/XgggT2h.jpg
These are not full size of course, just screenshots.

Colors are wrong and I'm pretty sure I know why, that's TODO.


Στις Τετ, 24 Ιουλ 2019 στις 12:04 μ.μ., ο/η Nick Renieris
 έγραψε:
>
> I have a small patch for basic gamma correction, it makes the dark
> images look much better.
> Should I send it here (if so, just as a single email reply to this?)
> or wait until this patchset is merged?
___
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 2/2] lavc/tiff: Decode embedded JPEGs in DNG images

2019-07-24 Thread Nick Renieris
I have a small patch for basic gamma correction, it makes the dark
images look much better.
Should I send it here (if so, just as a single email reply to this?)
or wait until this patchset is merged?
___
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 V1 2/3] doc/ffmpeg: Document dts_error_threshold option

2019-07-24 Thread Gyan



On 24-07-2019 01:38 PM, Michael Niedermayer wrote:

On Sun, Jul 21, 2019 at 10:31:36PM +0800, Jun Zhao wrote:

From: Jun Zhao 

Document dts_error_threshold option.

Signed-off-by: Jun Zhao 
---
  doc/ffmpeg.texi |2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
index cd35eb4..2152e62 100644
--- a/doc/ffmpeg.texi
+++ b/doc/ffmpeg.texi
@@ -1515,6 +1515,8 @@ Enable bitexact mode for (de)muxer and (de/en)coder
  Finish encoding when the shortest input stream ends.
  @item -dts_delta_threshold
  Timestamp discontinuity delta threshold.
+@item -dts_error_threshold
+Timestamp error delta threshold.

ok, though a bit terse

Yes, mention what this option actually does. Mention units and if 
applicable, range.


Gyan
___
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/rl2: set dimensions

2019-07-24 Thread Michael Niedermayer
On Wed, Jul 24, 2019 at 01:39:01AM +0100, Kieran Kunhya wrote:
> >
> > What was the cause of the slow decoding? Does this actually fix it, or
> > does it just swipe it "under the carpet"?
> > If someone ever found a sample with a different solution, how would they
> > know that they shouldn't just remove this again? Without any kind of
> > comment on the point of this call, people might assume it's pointless
> > nonsense.
> >
> 
> A significant proportion of these patches sweep the issue under the carpet.

If you know of a problem with a patch and or if you have a suggestion for
improving one, report the issue to the author of the patch.

Such general statments are not helping the project because noone can
fix a wide unspecific claim like that or even verify if the statement is true.

Which makes this just an ad hominem attack. Because it can very easily be
assigned to a human (who wrote all the patches) while making it impossible
to assign it to a technical issue that can be corrected or discussed.

Thanks

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In a rich man's house there is no place to spit but his face.
-- 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] [PATCH] avcodec/rl2: set dimensions

2019-07-24 Thread Michael Niedermayer
On Wed, Jul 24, 2019 at 01:29:26AM +0200, Reimar Döffinger wrote:
> 
> 
> On 22.07.2019, at 23:57, Michael Niedermayer  wrote:
> 
> > The dimensions are always 320x200 they are hardcoded in the demuxer.
> > Hardcode them instead in the decoder.
> > 
> > Fixes: Timeout (16sec -> 400ms)
> > Fixes: 
> > 15574/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_RL2_fuzzer-5158614072819712
> > 
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> > ---
> > libavcodec/rl2.c | 5 +
> > 1 file changed, 5 insertions(+)
> > 
> > diff --git a/libavcodec/rl2.c b/libavcodec/rl2.c
> > index 6662979c52..2d336a61e5 100644
> > --- a/libavcodec/rl2.c
> > +++ b/libavcodec/rl2.c
> > @@ -134,10 +134,15 @@ static av_cold int rl2_decode_init(AVCodecContext 
> > *avctx)
> > Rl2Context *s = avctx->priv_data;
> > int back_size;
> > int i;
> > +int ret;
> > 
> > s->avctx   = avctx;
> > avctx->pix_fmt = AV_PIX_FMT_PAL8;
> > 
> > +ret = ff_set_dimensions(avctx, 320, 200);
> > +if (ret < 0)
> > +return ret;
> 
> I really dislike these completely comment-less patches.
> So it seems this is only based on what our demuxer does.

No, the only specification available to me:
https://wiki.multimedia.cx/index.php/RL2
says this:
"Video Decoding
 The video is always encoded in 320x200 resolution." 

I will add this to the commit message of course in case no other
objections remain

 
> However does the format itself have any inherent size limitations?

yes, it conatins a video_base parameter which is 16bit and indexes into
the width*height array of pixels. suggesting that a size around 320x200
is the intended maximum which would use that parameter.


> What was the cause of the slow decoding? Does this actually fix it, or does 
> it just swipe it "under the carpet"?

The cause of slow decoding was a huge input resolution, precissely 64768x255
that also is far from the worst possible


> If someone ever found a sample with a different solution, how would they know 
> that they shouldn't just remove this again? Without any kind of comment on 
> the point of this call, people might assume it's pointless nonsense.

if some rl2 "2.0" is found which does store a resolution
and if that is reverse engeneered (these being obviously pre-requirements)
then the removial of the hardcoded size is probably not completely unreasonable

Thanks


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

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v8] Fix integer parameters size check in SDP fmtp line

2019-07-24 Thread Olivier Maignial
=== PROBLEM ===

I was trying to record h264 + aac streams from an RTSP server to mp4 file. 
using this command line:
ffmpeg -v verbose -y -i "rtsp:///my_resources" -codec copy -bsf:a 
aac_adtstoasc test.mp4

FFmpeg then fail to record audio and output this logs:
[rtsp @ 0xcda1f0] The profile-level-id field size is invalid (40)
[rtsp @ 0xcda1f0] Error parsing AU headers
...
[rtsp @ 0xcda1f0] Could not find codec parameters for stream 1 (Audio: aac, 
48000 Hz, 1 channels): unspecified sample format

In SDP provided by my RTSP server I had this fmtp line:
a=fmtp:98 streamType=5; profile-level-id=40; mode=AAC-hbr; config=1188; 
sizeLength=13; indexLength=3; indexDeltaLength=3;

In FFmpeg code, I found a check introduced by commit 
24130234cd9dd733116d17b724ea4c8e12ce097a. It disallows values greater than 32 
for fmtp line parameters.
RFC-4566 (SDP: Session Description Protocol) do not give any limit of size on 
interger parameters given in an fmtp line.

However, In RFC-6416 (RTP Payload Format for MPEG-4 Audio/Visual Streams) give 
examples of "profile-level-id" values for AAC, up to 55.

=== FIX ===

As each parameter may have its own min and max values
I propose to introduce a range for each parameter.
For this patch I used RFC-3640 and ISO/IEC 14496-1 as reference for validity 
ranges.

This patch fix my problem and I now can record my RTSP AAC stream to mp4.
It has passed the full fate tests suite sucessfully.

Signed-off-by: Olivier Maignial 
---
Changes v7 --> v8:
Indroduced a per parameter validity range 

 libavformat/rtpdec_mpeg4.c | 45 +
 1 file changed, 33 insertions(+), 12 deletions(-)

diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c
index 4f70599..08e5b98 100644
--- a/libavformat/rtpdec_mpeg4.c
+++ b/libavformat/rtpdec_mpeg4.c
@@ -70,6 +70,12 @@ typedef struct AttrNameMap {
 const char *str;
 uint16_ttype;
 uint32_toffset;
+
+/** Range for integer values */
+struct Range {
+int min;
+int max;
+} range;
 } AttrNameMap;
 
 /* All known fmtp parameters and the corresponding RTPAttrTypeEnum */
@@ -77,18 +83,24 @@ typedef struct AttrNameMap {
 #define ATTR_NAME_TYPE_STR 1
 static const AttrNameMap attr_names[] = {
 { "SizeLength",   ATTR_NAME_TYPE_INT,
-  offsetof(PayloadContext, sizelength) },
+  offsetof(PayloadContext, sizelength),
+  {0, 32} }, // SizeLength number of bits used to encode AU-size integer 
value
 { "IndexLength",  ATTR_NAME_TYPE_INT,
-  offsetof(PayloadContext, indexlength) },
+  offsetof(PayloadContext, indexlength),
+  {0, 32} }, // IndexLength number of bits used to encode AU-Index integer 
value
 { "IndexDeltaLength", ATTR_NAME_TYPE_INT,
-  offsetof(PayloadContext, indexdeltalength) },
+  offsetof(PayloadContext, indexdeltalength),
+  {0, 32} }, // IndexDeltaLength number of bits to encode AU-Index-delta 
integer value
 { "profile-level-id", ATTR_NAME_TYPE_INT,
-  offsetof(PayloadContext, profile_level_id) },
+  offsetof(PayloadContext, profile_level_id),
+  {INT32_MIN, INT32_MAX} }, // It differs depending on StreamType
 { "StreamType",   ATTR_NAME_TYPE_INT,
-  offsetof(PayloadContext, streamtype) },
+  offsetof(PayloadContext, streamtype),
+  {0x00, 0x3F} }, // Values from ISO/IEC 14496-1, 'StreamType Values' table
 { "mode", ATTR_NAME_TYPE_STR,
-  offsetof(PayloadContext, mode) },
-{ NULL, -1, -1 },
+  offsetof(PayloadContext, mode),
+   {0} },
+{ NULL, -1, -1, {0} },
 };
 
 static void close_context(PayloadContext *data)
@@ -289,15 +301,24 @@ static int parse_fmtp(AVFormatContext *s,
 for (i = 0; attr_names[i].str; ++i) {
 if (!av_strcasecmp(attr, attr_names[i].str)) {
 if (attr_names[i].type == ATTR_NAME_TYPE_INT) {
-int val = atoi(value);
-if (val > 32) {
+char *end_ptr = NULL;
+long long int val = strtoll(value, _ptr, 10);
+if (end_ptr == value || end_ptr[0] != '\0') {
 av_log(s, AV_LOG_ERROR,
-   "The %s field size is invalid (%d)\n",
-   attr, val);
+   "The %s field value is not a valid number: 
%s\n",
+   attr, value);
 return AVERROR_INVALIDDATA;
 }
+if (val < attr_names[i].range.min ||
+val > attr_names[i].range.max) {
+av_log(s, AV_LOG_ERROR,
+"fmtp field %s should be in range [%d,%d] 
(provided value: %lld)",
+attr, attr_names[i].range.min, 
attr_names[i].range.max, val);
+return  AVERROR_INVALIDDATA;
+}
+
  

Re: [FFmpeg-devel] [PATCH V1 2/3] doc/ffmpeg: Document dts_error_threshold option

2019-07-24 Thread Michael Niedermayer
On Sun, Jul 21, 2019 at 10:31:36PM +0800, Jun Zhao wrote:
> From: Jun Zhao 
> 
> Document dts_error_threshold option.
> 
> Signed-off-by: Jun Zhao 
> ---
>  doc/ffmpeg.texi |2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> index cd35eb4..2152e62 100644
> --- a/doc/ffmpeg.texi
> +++ b/doc/ffmpeg.texi
> @@ -1515,6 +1515,8 @@ Enable bitexact mode for (de)muxer and (de/en)coder
>  Finish encoding when the shortest input stream ends.
>  @item -dts_delta_threshold
>  Timestamp discontinuity delta threshold.
> +@item -dts_error_threshold
> +Timestamp error delta threshold.

ok, though a bit terse

thx

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

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/4] h264_mp4toannexb_bsf: Improve extradata overread checks

2019-07-24 Thread Michael Niedermayer
On Mon, Jul 22, 2019 at 05:27:13AM +0200, Andreas Rheinhardt wrote:
> 1. Currently during parsing the extradata, h264_mp4toannexb checks for
> overreads by adding the size of the current unit to the current position
> pointer and comparing this to the end position of the extradata. But
> pointer comparisons and pointer arithmetic is only defined if it does not
> exceed the object it is used on (one past the last element of an array
> is allowed, too). In practice, this might lead to overflows. Therefore
> the check has been changed.
> 2. The minimal size of an AVCDecoderConfigurationRecord is actually 7:
> Four bytes containing version, profile and level, one byte for length
> size and one byte each for the numbers of SPS and PPS. This has been
> changed. The byte for the number of PPS has been forgotten.

> 3. The earlier code also did not detect an error if the extradata ended
> directly after the last SPS in the SPS array (if any) although the
> number of PPS has to come afterwards. A check for this has been
> integrated into the general overread check.

what if there is no pps afterwards and instead in stream?
could this change break such streams ?


> 4. The earlier code also might overread when reading the size of the
> next unit. Given that this overread is not dangerous (the extradata is
> supposed to be padded), only a comment for it has been added; the error
> itself will be detected as part of the normal check for overreads.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/h264_mp4toannexb_bsf.c | 16 +---
>  1 file changed, 9 insertions(+), 7 deletions(-)

changes feel ok but this should be split as they are independant issues

thanks

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Whats the most studid thing your enemy could do ? Blow himself up
Whats the most studid thing you could do ? Give up your rights and
freedom because your enemy blew himself up.



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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 4/4] dnn: convert tf.pad to native model in python script, and load/execute it in the c code.

2019-07-24 Thread Guo, Yejun


> -Original Message-
> From: Guo, Yejun
> Sent: Tuesday, July 16, 2019 1:56 PM
> To: ffmpeg-devel@ffmpeg.org
> Cc: Guo, Yejun 
> Subject: [PATCH 4/4] dnn: convert tf.pad to native model in python script, and
> load/execute it in the c code.
> 
> since tf.pad is enabled, the conv2d(valid) changes back to its original 
> behavior.
> 
> Signed-off-by: Guo, Yejun 
> ---
>  libavfilter/dnn/dnn_backend_native.c| 35
> +
>  libavfilter/dnn/dnn_backend_native.h|  2 +-
>  tools/python/convert_from_tensorflow.py | 23 +-
>  3 files changed, 54 insertions(+), 6 deletions(-)
> 
> diff --git a/libavfilter/dnn/dnn_backend_native.c
> b/libavfilter/dnn/dnn_backend_native.c
> index 82e900b..09c583b 100644
> --- a/libavfilter/dnn/dnn_backend_native.c
> +++ b/libavfilter/dnn/dnn_backend_native.c
> @@ -25,6 +25,7 @@

this patch set asks for review, thanks.

I've locally finished more patches to improve dnn module, plan to send more 
them set by set, since the patches have dependency.

Just in case you are interested in my new patches, I've uploaded to 
https://github.com/guoyejun/ffmpeg/tree/dnn0716. 
for your convenient, I also copy the oneline log here for each patch (from 
newer to older) with 3 patch sets.

50a3353 fate: add unit test for dnn depth_to_space layer
af9e3ab dnn: separate depth_to_space layer from dnn_backend_native.c to a new 
file
41b97e4 fate: add unit test for dnn conv2d layer
4143485 dnn: separate conv2d layer from dnn_backend_native.c to a new file

870383e dnn: export operand info in python script and load in c code
650d576 dnn: change .model file format to put layer number at the end of file
d029bf8 dnn: introduce dnn operand (in c code) to hold operand infos within 
network

c9b9e1c doc/filters: update how to generate native model for derain filter
064aa45 convert_from_tensorflow.py: support conv2d with dilation
1c419a5 convert_from_tensorflow.py: add option to dump graph for visualization 
in tensorboard


___
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/4] h264_mp4toannexb: Remove unnecessary check

2019-07-24 Thread Michael Niedermayer
On Mon, Jul 22, 2019 at 05:27:12AM +0200, Andreas Rheinhardt wrote:
> There can be at most 31 SPS and 255 PPS in the mp4/Matroska extradata.
> Given that each has a size of at most 2^16-1, the length of the output
> derived from these parameter sets can never overflow an ordinary 32 bit
> integer. So use a simple int instead of uint64_t and remove the
> unnecessary check.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/h264_mp4toannexb_bsf.c | 8 +---
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/libavcodec/h264_mp4toannexb_bsf.c 
> b/libavcodec/h264_mp4toannexb_bsf.c
> index fb3f24ea40..8b2899f300 100644
> --- a/libavcodec/h264_mp4toannexb_bsf.c
> +++ b/libavcodec/h264_mp4toannexb_bsf.c
> @@ -68,7 +68,7 @@ static int h264_extradata_to_annexb(AVBSFContext *ctx, 
> const int padding)
>  {
>  H264BSFContext *s = ctx->priv_data;
>  uint16_t unit_size;
> -uint64_t total_size = 0;
> +int total_size  = 0;
>  uint8_t *out= NULL, unit_nb, sps_done = 0,
>   sps_seen   = 0, pps_seen = 0;
>  const uint8_t *extradata= ctx->par_in->extradata + 4;
> @@ -91,12 +91,6 @@ static int h264_extradata_to_annexb(AVBSFContext *ctx, 
> const int padding)
>  
>  unit_size   = AV_RB16(extradata);
>  total_size += unit_size + 4;
> -if (total_size > INT_MAX - padding) {
> -av_log(ctx, AV_LOG_ERROR,
> -   "Too big extradata size, corrupted stream or invalid 
> MP4/AVCC bitstream\n");
> -av_free(out);
> -return AVERROR(EINVAL);
> -}

this should be replaced by an av_assertX() to ensure that no future change
or extension exceeds the limits needed for the check to be not needed.

thanks

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

Elect your leaders based on what they did after the last election, not
based on what they say before an election.



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] avcodec: add a WavPack DSD decoder

2019-07-24 Thread Paul B Mahol
On 7/23/19, David Bryant  wrote:
> On 7/23/19 12:47 AM, Paul B Mahol wrote:
>> On 7/23/19, David Bryant  wrote:
>>> On 7/21/19 11:23 PM, Paul B Mahol wrote:
 On 7/22/19, David Bryant  wrote:
> Hi,
>
> As I promised late last year, here is a patch to add a WavPack DSD
> decoder.
>
> Thanks!
>
> -David Bryant
>
>
 Please correct me if I'm wrong, but why this uses new codec id?
 Apparently is also copies some logic from other files.
>>> Yes, I created a new codec ID for WavPack DSD. It would be possible to
>>> just
>>> piggyback on the existing WavPack codec by silently
>>> decimating the DSD to PCM, but that seemed weird and wrong to me. For one
>>> thing, the user would have no idea that the file was
>>> actually DSD and not high sample-rate PCM.
>>>
>>> Also, since regular WavPack has threading enabled but WavPack DSD can't
>>> (because of the dsd2pcm conversion) I would have to turn
>>> off threading for all WavPack (unless there's some way of making that
>>> conditional at runtime). It would also mean that regular
>>> WavPack would be dependent on the dsd2pcm component even if DSD was not
>>> required (not everyone needs DSD). And of course I was
>>> looking closely at the only other DSD codec in FFmpeg (DST) which has its
>>> own codec ID.
>>>
>>> Because regular WavPack PCM and DSD share the same block format and
>>> metadata
>>> structure, there is a little code that is shared
>>> between the two codecs (although they are no longer identical because of
>>> the
>>> threading difference). Is this a problem? I could
>>> combine the two codecs into one file and sprinkle in a few conditionals,
>>> but
>>> I don't think it would be as clean or clear (but
>>> might save a little duplicate code).
>>>
>>> That's my thinking, but obviously I am not as familiar with the FFmpeg
>>> philosophy as you guys.
>> Looking carefully at dsd2pcm code in your patch, it appears it would
>> work only with 1 or 2 channels.
>> Is this correct?
>
> Individual WavPack blocks can only be 1 or 2 channels. Multichannel files
> contains sequences of blocks which ends up creating
> multiple frame contexts. This was originally implemented for PCM WavPack
> years ago and so it "just works" for DSD WavPack
> because I didn't touch any of that.
>
> I have tested this extensively with multichannel DSD files and it works
> great, including seeking.

Than I fail to see why it could not work for multi-threading too.

>
>
>>
>> About multi-threading, as dsd2pcm can not be made multi-threaded then
>> just do locking when doing dsd2pcm to make it single threaded and
>> update other code as necessary.
>
> It's not really that dsd2pcm cannot be multi-threaded, it's that the audio
> frames cannot be converted to PCM independently or
> you'll get a click at the frame boundaries. Maybe we're talking about the
> same thing, but I don't see how a lock would fix this.
> In any event, could you please show me an example of doing that?

The ffmpeg have functionality to await progress of some work, which
allows doing some stuff single threaded.
For example look at apng decoder in libavcodec/pngdec.c
See ff_thread_await/report_progress and .update_thread_context/.init_thread_copy
and relevant documentation of mentioned functions.

>
>
>>
>> I still see no valid reason to add separate codec id. Add another
>> wavpack profile if you wish to help users know the difference between
>> the two.
>
> Again I apologize for not being too familiar with FFmpeg internals. What
> would another wavpack "profile" be? As long as the user
> (or caller) has some way to differentiate between DSD and PCM files without
> parsing the file themselves I'm happy, but what
> exactly is the hesitation here? Are codec IDs a limited resource? It seems
> like an obvious win to me.

For profile explanation look how other codecs do it, like
libavcodec/proresdec2.c
Copy pasting other code is sure never win.

>
> -David
>
>
>> ___
>> 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] libavcodec: add timer bitstream filter

2019-07-24 Thread Andreas Håkon
Hi,

‐‐‐ Original Message ‐‐‐
On Friday, 28 de June de 2019 10:44, Andreas Håkon 
 wrote:

> Hi,
> This is a refined version of the initial version for the new “timer” BSF.
> Supersedes: https://patchwork.ffmpeg.org/patch/13699/

Still waiting to receive comments (or acceptance).

Regards.
A.H.

---
___
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/mips: Avoid instruction exception caused by gssqc1/gslqc1.

2019-07-24 Thread Shiyou Yin
>Why is "block" not aligned? Does the code for other architectures also use 
>unaligned instructions for
>these?

Thank you for reminding me. After checking the struct H264SliceContext and 
function call process, 'block' is find out as 16-bit aligned.
There are some refines in this patch, I will upload them in a new patch and 
only keep the following changes in this patch(V2).

>> diff --git a/libavcodec/mips/simple_idct_mmi.c 
>> b/libavcodec/mips/simple_idct_mmi.c
>> index 7f4bb74..f54f9ea 100644
>> --- a/libavcodec/mips/simple_idct_mmi.c
>> +++ b/libavcodec/mips/simple_idct_mmi.c
>> @@ -39,7 +39,7 @@
>> #define COL_SHIFT 20
>> #define DC_SHIFT 3
>>
>> -DECLARE_ALIGNED(8, const int16_t, W_arr)[46] = {
>> +DECLARE_ALIGNED(16, const int16_t, W_arr)[46] = {
>> W4,  W2,  W4,  W6,
>> W1,  W3,  W5,  W7,
>> W4,  W6, -W4, -W2,
>> diff --git a/libavutil/mips/mmiutils.h b/libavutil/mips/mmiutils.h
>> index 05f6b31..bfa6d8b 100644
>> --- a/libavutil/mips/mmiutils.h
>> +++ b/libavutil/mips/mmiutils.h
>> @@ -205,7 +205,7 @@
>>  * backup register
>>  */
>> #define BACKUP_REG \
>> -  double temp_backup_reg[8];\
>> +  double __attribute__ ((aligned (16))) temp_backup_reg[8]; \
>>   if (_MIPS_SIM == _ABI64)  \
>> __asm__ volatile (  \
>>   "gssqc1   $f25,  $f24,   0x00(%[temp])  \n\t" \
>> --
>> 2.1.0


>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] [PATCH v2] avutil/mips: Avoid instruction exception caused by gssqc1/gslqc1.

2019-07-24 Thread Shiyou Yin
Ensure the address accesed by gssqc1/gslqc1 are 16-bits memory-aligned.
---
 libavcodec/mips/simple_idct_mmi.c | 2 +-
 libavutil/mips/mmiutils.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/mips/simple_idct_mmi.c 
b/libavcodec/mips/simple_idct_mmi.c
index 7f4bb74..73d797f 100644
--- a/libavcodec/mips/simple_idct_mmi.c
+++ b/libavcodec/mips/simple_idct_mmi.c
@@ -39,7 +39,7 @@
 #define COL_SHIFT 20
 #define DC_SHIFT 3
 
-DECLARE_ALIGNED(8, const int16_t, W_arr)[46] = {
+DECLARE_ALIGNED(16, const int16_t, W_arr)[46] = {
 W4,  W2,  W4,  W6,
 W1,  W3,  W5,  W7,
 W4,  W6, -W4, -W2,
diff --git a/libavutil/mips/mmiutils.h b/libavutil/mips/mmiutils.h
index 05f6b31..bfa6d8b 100644
--- a/libavutil/mips/mmiutils.h
+++ b/libavutil/mips/mmiutils.h
@@ -205,7 +205,7 @@
  * backup register
  */
 #define BACKUP_REG \
-  double temp_backup_reg[8];\
+  double __attribute__ ((aligned (16))) temp_backup_reg[8]; \
   if (_MIPS_SIM == _ABI64)  \
 __asm__ volatile (  \
   "gssqc1   $f25,  $f24,   0x00(%[temp])  \n\t" \
-- 
2.1.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] avcodec/rl2: set dimensions

2019-07-24 Thread Reimar Döffinger
On 24.07.2019, at 02:39, Kieran Kunhya  wrote:

>> 
>> What was the cause of the slow decoding? Does this actually fix it, or
>> does it just swipe it "under the carpet"?
>> If someone ever found a sample with a different solution, how would they
>> know that they shouldn't just remove this again? Without any kind of
>> comment on the point of this call, people might assume it's pointless
>> nonsense.
>> 
> 
> A significant proportion of these patches sweep the issue under the carpet.

Which is not necessarily the wrong choice.
But by leaving no documentation a lot of the time spend on writing the patches 
is wasted since the knowledge gained is just lost, maintainers need to 
"reverse-engineer" them etc.

> Not to mention the swathes of annoyed developers

And let's not turn this into a conflict, I just tried to give clear feedback 
why I am unhappy
with some of these patches even though they may be worthwhile still.
> 

Hopefully some of that also captures reasons others feel unhappy (and it would 
be nice
if the concerns could be raised more constructively, though I do acknowledge it 
can be hard).
___
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".