Re: [FFmpeg-devel] [PATCH] x86/yuv2rgb: fix crashes when storing data on unaligned buffers

2020-07-13 Thread Carl Eugen Hoyos


> Am 13.07.2020 um 02:33 schrieb James Almer :
> 
> Regression since fc6a5883d6af8cae0e96af84dda0ad74b360a084 on SSSE3 enabled
> CPUs.

FFmpeg does not require aligned output buffers on x86_64?

> Fixes ticket #8747

Another issue is described in this ticket that implies an overwrite: Can you 
reproduce?

Thank you, Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] Bug in FFMPEG video filter TINTERLACE needs fixing

2020-07-13 Thread Paul B Mahol
You have setfield and similar filters that can be used before this filter.

On 7/12/20, Ben Hutchinson  wrote:
> Problem is it's not as simple as just patching it to start at field 1
> instead of field 2. It really depends on your signal source. If your signal
> source recorded all lines that contain at least some image signal (even if
> only half the line contains image signal), then the top line of the image
> is the first line of field 2 (the even field). However, if your signal
> source only recorded lines where the entire line contains image signal then
> the top line of the image is the first line of field 1 (the odd field).
> When using the TINTERLACE video filter (at least in MERGEX2 mode), one
> actually needs to be able to select which field comes first (odd field
> first, or even field first). This unfortunately is a much more in-depth
> patch to the software than just hardcoding a different starting field. You
> will need to add commandline option for the TINTERLACE filter in FFMPEG in
> order to allow for user-selection of OFF or EFF.
>
> On Sun, Jul 12, 2020 at 9:10 AM Paul B Mahol  wrote:
>
>> On 7/12/20, Ben Hutchinson  wrote:
>> > I was assuming it started from 1, because in the NTSC video standard
>> (which
>> > is of course where the concept of interlaced video originated), the
>> > first
>> > field is called field 1. Field 1 contains the first full line of
>> displayed
>> > video (though technically field 2 contains the first image data, one
>> > line
>> > above the first displayed line of field 1, although it's only the right
>> > half of that line).
>>
>> I sent patch to fix this.
>>
>> >
>> > On Sat, Jul 11, 2020 at 4:07 AM Paul B Mahol  wrote:
>> >
>> >> On 7/11/20, Ben Hutchinson  wrote:
>> >> > I was reading it directly from the official FFMPEG website.
>> >> > https://ffmpeg.org/ffmpeg-filters.html
>> >> > All the info about the TINTERLACE filter I got from reading the info
>> >> there.
>> >> >
>> >>
>> >> OK, lets try again, where have you read that first frame is always an
>> >> odd frame, one with 1 number.
>> >> FFmpeg counts from 0.
>> >>
>> >> > On Fri, Jul 10, 2020 at 2:14 AM Paul B Mahol 
>> wrote:
>> >> >
>> >> >> On 7/8/20, Ben Hutchinson  wrote:
>> >> >> > According to the documentation on the TINTERLACE video filter, the
>> >> >> > filter
>> >> >> > mode called MERGEX2 will "Move odd frames into the upper field,
>> even
>> >> >> > into
>> >> >> > the lower field, generating a double height frame at same frame
>> >> >> > rate."
>> >> >> But
>> >> >> > it doesn't do this, at least in some cases (not sure about all
>> >> >> > cases).
>> >> >> The
>> >> >> > first frame in a sequence should be considered frame one (an odd
>> >> frame)
>> >> >> for
>> >> >>
>> >> >> Where is this written?
>> >> >>
>> >> >> > the purpose of this interlacing algorithm. However, that is not
>> >> >> > what's
>> >> >> > happening in my experience. At least with raw video (using "-f
>> >> >> > rawvideo")
>> >> >> > it's treating the first frame as frame zero (an even frame) and
>> thus
>> >> my
>> >> >> > first frame (which contains top-field data) ends up getting put
>> into
>> >> the
>> >> >> > bottom-field of the output video, and this is messing up the
>> output.
>> >> >> Please
>> >> >> > fix 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".
>> >> >> ___
>> >> >> 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".
>> > ___
>> > 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

[FFmpeg-devel] [PATCH 2/2] SpeedHQ encoder

2020-07-13 Thread Jean-Baptiste Kempf
This is heavily based on MPEG-2 encoder, of course.
---
 Changelog  |  1 +
 doc/general.texi   |  2 +-
 libavcodec/Makefile|  1 +
 libavcodec/allcodecs.c |  1 +
 libavcodec/mpegutils.h |  1 +
 libavcodec/mpegvideo.h |  3 +++
 libavcodec/mpegvideo_enc.c | 52 +++---
 libavcodec/version.h   |  2 +-
 8 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/Changelog b/Changelog
index 49fc260458..8f3dcbec87 100644
--- a/Changelog
+++ b/Changelog
@@ -7,6 +7,7 @@ version :
 - PGX decoder
 - chromanr video filter
 - VDPAU accelerated HEVC 10/12bit decoding
+- SpeedHQ encoder
 
 
 version 4.3:
diff --git a/doc/general.texi b/doc/general.texi
index 53d556c56c..7ab1c61267 100644
--- a/doc/general.texi
+++ b/doc/general.texi
@@ -971,7 +971,7 @@ following image formats are supported:
 @item MPEG-4 part 2 Microsoft variant version 1  @tab @tab  X
 @item MPEG-4 part 2 Microsoft variant version 2  @tab  X  @tab  X
 @item MPEG-4 part 2 Microsoft variant version 3  @tab  X  @tab  X
-@item Newtek SpeedHQ   @tab @tab  X
+@item Newtek SpeedHQ   @tab  X  @tab  X
 @item Nintendo Gamecube THP video  @tab @tab  X
 @item NotchLC@tab @tab  X
 @item NuppelVideo/RTjpeg @tab @tab  X
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 18353da549..52445130f4 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -614,6 +614,7 @@ OBJS-$(CONFIG_SONIC_DECODER)   += sonic.o
 OBJS-$(CONFIG_SONIC_ENCODER)   += sonic.o
 OBJS-$(CONFIG_SONIC_LS_ENCODER)+= sonic.o
 OBJS-$(CONFIG_SPEEDHQ_DECODER) += speedhq.o mpeg12.o mpeg12data.o 
simple_idct.o
+OBJS-$(CONFIG_SPEEDHQ_ENCODER) += speedhq.o mpeg12data.o speedhqenc.o
 OBJS-$(CONFIG_SP5X_DECODER)+= sp5xdec.o
 OBJS-$(CONFIG_SRGC_DECODER)+= mscc.o
 OBJS-$(CONFIG_SRT_DECODER) += srtdec.o ass.o htmlsubtitles.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index a5048290f7..b542910303 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -290,6 +290,7 @@ extern AVCodec ff_snow_encoder;
 extern AVCodec ff_snow_decoder;
 extern AVCodec ff_sp5x_decoder;
 extern AVCodec ff_speedhq_decoder;
+extern AVCodec ff_speedhq_encoder;
 extern AVCodec ff_srgc_decoder;
 extern AVCodec ff_sunrast_encoder;
 extern AVCodec ff_sunrast_decoder;
diff --git a/libavcodec/mpegutils.h b/libavcodec/mpegutils.h
index 1ed21c19be..81f0b73bb1 100644
--- a/libavcodec/mpegutils.h
+++ b/libavcodec/mpegutils.h
@@ -125,6 +125,7 @@ enum OutputFormat {
 FMT_H261,
 FMT_H263,
 FMT_MJPEG,
+FMT_SPEEDHQ,
 };
 
 
diff --git a/libavcodec/mpegvideo.h b/libavcodec/mpegvideo.h
index 29e692f245..974c71b6bd 100644
--- a/libavcodec/mpegvideo.h
+++ b/libavcodec/mpegvideo.h
@@ -444,6 +444,9 @@ typedef struct MpegEncContext {
 int inter_intra_pred;
 int mspel;
 
+/* SpeedHQ specific */
+int slice_start;
+
 /* decompression specific */
 GetBitContext gb;
 
diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index c3ef40556a..221079452c 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -51,6 +51,7 @@
 #include "mathops.h"
 #include "mpegutils.h"
 #include "mjpegenc.h"
+#include "speedhqenc.h"
 #include "msmpeg4.h"
 #include "pixblockdsp.h"
 #include "qpeldsp.h"
@@ -326,6 +327,15 @@ av_cold int ff_mpv_encode_init(AVCodecContext *avctx)
 return AVERROR(EINVAL);
 }
 break;
+case AV_CODEC_ID_SPEEDHQ:
+if (avctx->pix_fmt != AV_PIX_FMT_YUV420P &&
+avctx->pix_fmt != AV_PIX_FMT_YUV422P &&
+avctx->pix_fmt != AV_PIX_FMT_YUV444P) {
+av_log(avctx, AV_LOG_ERROR,
+   "only YUV420/YUV422/YUV444 are supported (no alpha support 
yet)\n");
+return AVERROR(EINVAL);
+}
+break;
 default:
 if (avctx->pix_fmt != AV_PIX_FMT_YUV420P) {
 av_log(avctx, AV_LOG_ERROR, "only YUV420 is supported\n");
@@ -732,7 +742,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
 //return -1;
 }
 
-if (s->mpeg_quant || s->codec_id == AV_CODEC_ID_MPEG1VIDEO || s->codec_id 
== AV_CODEC_ID_MPEG2VIDEO || s->codec_id == AV_CODEC_ID_MJPEG || 
s->codec_id==AV_CODEC_ID_AMV) {
+if (s->mpeg_quant || s->codec_id == AV_CODEC_ID_MPEG1VIDEO || s->codec_id 
== AV_CODEC_ID_MPEG2VIDEO || s->codec_id == AV_CODEC_ID_MJPEG || 
s->codec_id==AV_CODEC_ID_AMV || s->codec_id == AV_CODEC_ID_SPEEDHQ) {
 // (a + x * 3 / 8) / x
 s->intra_quant_bias = 3 << (QUANT_BIAS_SHIFT - 3);
 s->inter_quant_bias = 0;
@@ -783,6 +793,16 @@ FF_ENABLE_DEPRECATION_WARNINGS
 avctx->delay = 0;
 s->low_delay = 1;
 break;
+case AV_CODEC_ID_SPEEDHQ:
+s->out_format = FMT_SPEEDHQ;
+s->intra_only = 1; /* force intra only for SHQ */
+if (!CONFIG_SPEEDHQ_ENCODER)
+return AVERROR_E

[FFmpeg-devel] [PATCH 1/2] mpeg2: Renaming functions around init_uni_ac_vlc

2020-07-13 Thread Jean-Baptiste Kempf
We need to export init_uni_ac_vlc to init_uni_ac_vlc and therefore need
renaming ff_init_uni_ac_vlc to ff_init_uni_ac_huffman for the following
patch.
---
 libavcodec/mjpegenc.c| 4 ++--
 libavcodec/mjpegenc_common.c | 6 +++---
 libavcodec/mjpegenc_common.h | 2 +-
 libavcodec/mpeg12.h  | 2 ++
 libavcodec/mpeg12enc.c   | 6 +++---
 libavcodec/speedhq.c | 2 +-
 6 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/libavcodec/mjpegenc.c b/libavcodec/mjpegenc.c
index 56ccbc5fb1..eaa739306a 100644
--- a/libavcodec/mjpegenc.c
+++ b/libavcodec/mjpegenc.c
@@ -106,8 +106,8 @@ av_cold int ff_mjpeg_encode_init(MpegEncContext *s)
  avpriv_mjpeg_bits_ac_chrominance,
  avpriv_mjpeg_val_ac_chrominance);
 
-ff_init_uni_ac_vlc(m->huff_size_ac_luminance,   m->uni_ac_vlc_len);
-ff_init_uni_ac_vlc(m->huff_size_ac_chrominance, m->uni_chroma_ac_vlc_len);
+ff_init_uni_ac_huffman(m->huff_size_ac_luminance,   m->uni_ac_vlc_len);
+ff_init_uni_ac_huffman(m->huff_size_ac_chrominance, 
m->uni_chroma_ac_vlc_len);
 s->intra_ac_vlc_length  =
 s->intra_ac_vlc_last_length = m->uni_ac_vlc_len;
 s->intra_chroma_ac_vlc_length  =
diff --git a/libavcodec/mjpegenc_common.c b/libavcodec/mjpegenc_common.c
index 3038ebde6e..fed32efc67 100644
--- a/libavcodec/mjpegenc_common.c
+++ b/libavcodec/mjpegenc_common.c
@@ -36,7 +36,7 @@
 #include "mjpegenc_huffman.h"
 #include "mjpeg.h"
 
-av_cold void ff_init_uni_ac_vlc(const uint8_t huff_size_ac[256], uint8_t 
*uni_ac_vlc_len)
+av_cold void ff_init_uni_ac_huffman(const uint8_t huff_size_ac[256], uint8_t 
*uni_ac_vlc_len)
 {
 int i;
 
@@ -551,8 +551,8 @@ int ff_mjpeg_encode_stuffing(MpegEncContext *s)
 
 // Replace the VLCs with the optimal ones.
 // The default ones may be used for trellis during quantization.
-ff_init_uni_ac_vlc(m->huff_size_ac_luminance,   m->uni_ac_vlc_len);
-ff_init_uni_ac_vlc(m->huff_size_ac_chrominance, 
m->uni_chroma_ac_vlc_len);
+ff_init_uni_ac_huffman(m->huff_size_ac_luminance,   m->uni_ac_vlc_len);
+ff_init_uni_ac_huffman(m->huff_size_ac_chrominance, 
m->uni_chroma_ac_vlc_len);
 s->intra_ac_vlc_length  =
 s->intra_ac_vlc_last_length = m->uni_ac_vlc_len;
 s->intra_chroma_ac_vlc_length  =
diff --git a/libavcodec/mjpegenc_common.h b/libavcodec/mjpegenc_common.h
index e8698d18c6..c24fee0b4e 100644
--- a/libavcodec/mjpegenc_common.h
+++ b/libavcodec/mjpegenc_common.h
@@ -41,6 +41,6 @@ void ff_mjpeg_init_hvsample(AVCodecContext *avctx, int 
hsample[4], int vsample[4
 void ff_mjpeg_encode_dc(PutBitContext *pb, int val,
 uint8_t *huff_size, uint16_t *huff_code);
 
-av_cold void ff_init_uni_ac_vlc(const uint8_t huff_size_ac[256], uint8_t 
*uni_ac_vlc_len);
+av_cold void ff_init_uni_ac_huffman(const uint8_t huff_size_ac[256], uint8_t 
*uni_ac_vlc_len);
 
 #endif /* AVCODEC_MJPEGENC_COMMON_H */
diff --git a/libavcodec/mpeg12.h b/libavcodec/mpeg12.h
index 1ec99f17e1..f01d7197ff 100644
--- a/libavcodec/mpeg12.h
+++ b/libavcodec/mpeg12.h
@@ -77,4 +77,6 @@ void ff_mpeg12_find_best_frame_rate(AVRational frame_rate,
 int *code, int *ext_n, int *ext_d,
 int nonstandard);
 
+void ff_init_uni_ac_vlc(RLTable *rl, uint8_t *uni_ac_vlc_len);
+
 #endif /* AVCODEC_MPEG12_H */
diff --git a/libavcodec/mpeg12enc.c b/libavcodec/mpeg12enc.c
index 9fbbcef607..c33e20604e 100644
--- a/libavcodec/mpeg12enc.c
+++ b/libavcodec/mpeg12enc.c
@@ -64,7 +64,7 @@ static int8_t  mpeg1_max_level[2][64];
 
 #define A53_MAX_CC_COUNT 0x1f
 
-static av_cold void init_uni_ac_vlc(RLTable *rl, uint8_t *uni_ac_vlc_len)
+av_cold void ff_init_uni_ac_vlc(RLTable *rl, uint8_t *uni_ac_vlc_len)
 {
 int i;
 
@@ -1054,9 +1054,9 @@ av_cold void ff_mpeg1_encode_init(MpegEncContext *s)
 mpeg1_index_run[0][i] = ff_rl_mpeg1.index_run[0][i];
 }
 
-init_uni_ac_vlc(&ff_rl_mpeg1, uni_mpeg1_ac_vlc_len);
+ff_init_uni_ac_vlc(&ff_rl_mpeg1, uni_mpeg1_ac_vlc_len);
 if (s->intra_vlc_format)
-init_uni_ac_vlc(&ff_rl_mpeg2, uni_mpeg2_ac_vlc_len);
+ff_init_uni_ac_vlc(&ff_rl_mpeg2, uni_mpeg2_ac_vlc_len);
 
 /* build unified dc encoding tables */
 for (i = -255; i < 256; i++) {
diff --git a/libavcodec/speedhq.c b/libavcodec/speedhq.c
index b834b79f28..86eb725398 100644
--- a/libavcodec/speedhq.c
+++ b/libavcodec/speedhq.c
@@ -132,7 +132,7 @@ static const uint8_t speedhq_run[121] = {
 31,
 };
 
-static RLTable ff_rl_speedhq = {
+RLTable ff_rl_speedhq = {
 121,
 121,
 (const uint16_t (*)[])speedhq_vlc,
-- 
2.27.0

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

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

[FFmpeg-devel] [PATCH 2/2] SpeedHQ encoder

2020-07-13 Thread Jean-Baptiste Kempf
This is heavily based on MPEG-2 encoder, of course.
---
 Changelog  |   1 +
 doc/general.texi   |   2 +-
 libavcodec/Makefile|   1 +
 libavcodec/allcodecs.c |   1 +
 libavcodec/mpegutils.h |   1 +
 libavcodec/mpegvideo.h |   3 +
 libavcodec/mpegvideo_enc.c |  52 +-
 libavcodec/speedhqenc.c| 316 +
 libavcodec/speedhqenc.h|  47 ++
 libavcodec/version.h   |   2 +-
 10 files changed, 421 insertions(+), 5 deletions(-)
 create mode 100644 libavcodec/speedhqenc.c
 create mode 100644 libavcodec/speedhqenc.h

diff --git a/Changelog b/Changelog
index 49fc260458..8f3dcbec87 100644
--- a/Changelog
+++ b/Changelog
@@ -7,6 +7,7 @@ version :
 - PGX decoder
 - chromanr video filter
 - VDPAU accelerated HEVC 10/12bit decoding
+- SpeedHQ encoder
 
 
 version 4.3:
diff --git a/doc/general.texi b/doc/general.texi
index 53d556c56c..7ab1c61267 100644
--- a/doc/general.texi
+++ b/doc/general.texi
@@ -971,7 +971,7 @@ following image formats are supported:
 @item MPEG-4 part 2 Microsoft variant version 1  @tab @tab  X
 @item MPEG-4 part 2 Microsoft variant version 2  @tab  X  @tab  X
 @item MPEG-4 part 2 Microsoft variant version 3  @tab  X  @tab  X
-@item Newtek SpeedHQ   @tab @tab  X
+@item Newtek SpeedHQ   @tab  X  @tab  X
 @item Nintendo Gamecube THP video  @tab @tab  X
 @item NotchLC@tab @tab  X
 @item NuppelVideo/RTjpeg @tab @tab  X
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 18353da549..52445130f4 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -614,6 +614,7 @@ OBJS-$(CONFIG_SONIC_DECODER)   += sonic.o
 OBJS-$(CONFIG_SONIC_ENCODER)   += sonic.o
 OBJS-$(CONFIG_SONIC_LS_ENCODER)+= sonic.o
 OBJS-$(CONFIG_SPEEDHQ_DECODER) += speedhq.o mpeg12.o mpeg12data.o 
simple_idct.o
+OBJS-$(CONFIG_SPEEDHQ_ENCODER) += speedhq.o mpeg12data.o speedhqenc.o
 OBJS-$(CONFIG_SP5X_DECODER)+= sp5xdec.o
 OBJS-$(CONFIG_SRGC_DECODER)+= mscc.o
 OBJS-$(CONFIG_SRT_DECODER) += srtdec.o ass.o htmlsubtitles.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index a5048290f7..b542910303 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -290,6 +290,7 @@ extern AVCodec ff_snow_encoder;
 extern AVCodec ff_snow_decoder;
 extern AVCodec ff_sp5x_decoder;
 extern AVCodec ff_speedhq_decoder;
+extern AVCodec ff_speedhq_encoder;
 extern AVCodec ff_srgc_decoder;
 extern AVCodec ff_sunrast_encoder;
 extern AVCodec ff_sunrast_decoder;
diff --git a/libavcodec/mpegutils.h b/libavcodec/mpegutils.h
index 1ed21c19be..81f0b73bb1 100644
--- a/libavcodec/mpegutils.h
+++ b/libavcodec/mpegutils.h
@@ -125,6 +125,7 @@ enum OutputFormat {
 FMT_H261,
 FMT_H263,
 FMT_MJPEG,
+FMT_SPEEDHQ,
 };
 
 
diff --git a/libavcodec/mpegvideo.h b/libavcodec/mpegvideo.h
index 29e692f245..974c71b6bd 100644
--- a/libavcodec/mpegvideo.h
+++ b/libavcodec/mpegvideo.h
@@ -444,6 +444,9 @@ typedef struct MpegEncContext {
 int inter_intra_pred;
 int mspel;
 
+/* SpeedHQ specific */
+int slice_start;
+
 /* decompression specific */
 GetBitContext gb;
 
diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index c3ef40556a..221079452c 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -51,6 +51,7 @@
 #include "mathops.h"
 #include "mpegutils.h"
 #include "mjpegenc.h"
+#include "speedhqenc.h"
 #include "msmpeg4.h"
 #include "pixblockdsp.h"
 #include "qpeldsp.h"
@@ -326,6 +327,15 @@ av_cold int ff_mpv_encode_init(AVCodecContext *avctx)
 return AVERROR(EINVAL);
 }
 break;
+case AV_CODEC_ID_SPEEDHQ:
+if (avctx->pix_fmt != AV_PIX_FMT_YUV420P &&
+avctx->pix_fmt != AV_PIX_FMT_YUV422P &&
+avctx->pix_fmt != AV_PIX_FMT_YUV444P) {
+av_log(avctx, AV_LOG_ERROR,
+   "only YUV420/YUV422/YUV444 are supported (no alpha support 
yet)\n");
+return AVERROR(EINVAL);
+}
+break;
 default:
 if (avctx->pix_fmt != AV_PIX_FMT_YUV420P) {
 av_log(avctx, AV_LOG_ERROR, "only YUV420 is supported\n");
@@ -732,7 +742,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
 //return -1;
 }
 
-if (s->mpeg_quant || s->codec_id == AV_CODEC_ID_MPEG1VIDEO || s->codec_id 
== AV_CODEC_ID_MPEG2VIDEO || s->codec_id == AV_CODEC_ID_MJPEG || 
s->codec_id==AV_CODEC_ID_AMV) {
+if (s->mpeg_quant || s->codec_id == AV_CODEC_ID_MPEG1VIDEO || s->codec_id 
== AV_CODEC_ID_MPEG2VIDEO || s->codec_id == AV_CODEC_ID_MJPEG || 
s->codec_id==AV_CODEC_ID_AMV || s->codec_id == AV_CODEC_ID_SPEEDHQ) {
 // (a + x * 3 / 8) / x
 s->intra_quant_bias = 3 << (QUANT_BIAS_SHIFT - 3);
 s->inter_quant_bias = 0;
@@ -783,6 +793,16 @@ FF_ENABLE_DEPRECATION_WARNINGS
 avctx->delay = 0;
 s->low_delay = 1;
 break;
+case AV_CODEC

Re: [FFmpeg-devel] [PATCH] x86/yuv2rgb: fix crashes when storing data on unaligned buffers

2020-07-13 Thread James Almer
On 7/13/2020 4:02 AM, Carl Eugen Hoyos wrote:
> 
> 
>> Am 13.07.2020 um 02:33 schrieb James Almer :
>>
>> Regression since fc6a5883d6af8cae0e96af84dda0ad74b360a084 on SSSE3 enabled
>> CPUs.
> 
> FFmpeg does not require aligned output buffers on x86_64?

There's no alignment requirement mentioned anywhere in swscale.h, which
explains why every other asm function also uses unaligned mov
instructions when handling src and dst buffers.

> 
>> Fixes ticket #8747
> 
> Another issue is described in this ticket that implies an overwrite: Can you 
> reproduce?

No, I haven't tried that one under valgrind.

> 
> Thank you, Carl Eugen
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
> 

___
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 v08.01 2/3] fbtile tile/detile, hwcontext_drm detile NonLinear

2020-07-13 Thread Lynne
Jul 12, 2020, 18:21 by hanish...@gmail.com:

> ** fbtile cpu based framebuffer tile/detile helpers
>
> diff --git a/Changelog b/Changelog
> index 20ba03ae8b..0b48858da7 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -6,6 +6,8 @@ version :
>  - MacCaption demuxer
>  - PGX decoder
>  - kmsgrab GetFB2 format_modifier, if user doesnt specify
> +- fbtile cpu based framebuffer tile/detile helpers (Intel TileX|Y|Yf)
> +- hwcontext_drm detiles non linear layouts, if possible
>

Remove upper changelog entry (fbtile). Its purely internal.

 

> +#include "config.h"
> +#include "avutil.h"
> +#include "common.h"
> +#include "fbtile.h"
> +#if CONFIG_LIBDRM
> +#include 
> +#endif
> +
> +
> +/**
> + * Ok return value
> + */
> +#define FBT_OK 0
>

Remove this.



> +enum FFFBTileLayout ff_fbtile_getlayoutid(enum FFFBTileFamily family, 
> uint64_t familyTileType)
> +{
> +enum FFFBTileLayout layout = FF_FBTILE_UNKNOWN;
> +
> +switch(family) {
> +case FF_FBTILE_FAMILY_DRM:
> +#if CONFIG_LIBDRM
> +switch(familyTileType) {
> +case DRM_FORMAT_MOD_LINEAR:
> +layout = FF_FBTILE_NONE;
> +break;
> +case I915_FORMAT_MOD_X_TILED:
> +layout = FF_FBTILE_INTEL_XGEN9;
> +break;
> +case I915_FORMAT_MOD_Y_TILED:
> +layout = FF_FBTILE_INTEL_YGEN9;
> +break;
> +case I915_FORMAT_MOD_Yf_TILED:
> +layout = FF_FBTILE_INTEL_YF;
> +break;
> +default:
> +layout = FF_FBTILE_UNKNOWN;
> +break;
> +}
> +#else
> +av_log(NULL, AV_LOG_WARNING, "fbtile:getlayoutid: family[%d] 
> familyTileType[%ld]\n", family, familyTileType);
> +#endif
> +break;
> +default:
> +av_log(NULL, AV_LOG_WARNING, "fbtile:getlayoutid: unknown family[%d] 
> familyTileType[%ld]\n", family, familyTileType);
> +}
> +av_log(NULL, AV_LOG_VERBOSE, "fbtile:getlayoutid: family[%d] 
> familyTileType[%ld] maps to layoutid[%d]\n", family, familyTileType, layout);
> +return layout;
> +}
>

Remove entire function and work using libdrm's modifier defines.
There's absolutely no point in modularity because like I said, no other API is 
this terminally
bad to require detiling.



> +
> +
> +/**
> + * TileWalk Direction Change Entry
> + * Used to specify the tile walking of subtiles within a tile.
> + */
> +struct FBTWDirChange {
> +int posOffset;
> +int xDelta;
> +int yDelta;
> +};
> +
> +
> +/**
> + * TileWalk, Contains info required for a given tile walking.
> + *
> + * @field bytesPerPixel the bytes per pixel for the image
> + * @field subTileWidth the width of subtile within the tile, in pixels
> + * @field subTileHeight the height of subtile within the tile, in pixels
> + * @field tileWidth the width of the tile, in pixels
> + * @field tileHeight the height of the tile, in pixels
> + * @field numDirChanges the number of dir changes involved in tile walk
> + * @field dirChanges the array of dir changes for the tile walk required
> + */
> +struct FBTileWalk {
> +int bytesPerPixel;
> +int subTileWidth, subTileHeight;
> +int tileWidth, tileHeight;
> +int numDirChanges;
> +struct FBTWDirChange dirChanges[];
> +};
> +
> +
> +/**
> + * Settings for Intel Tile-Yf framebuffer layout.
> + * May need to swap the 4 pixel wide subtile, have to check doc bit more
> + */
> +static struct FBTileWalk tyfTileWalk = {
> +.bytesPerPixel = 4,
> +.subTileWidth = 4, .subTileHeight = 8,
> +.tileWidth = 32, .tileHeight = 32,
> +.numDirChanges = 6,
> +.dirChanges = { {8, 4, 0}, {16, -4, 8}, {32, 4, -8}, 
> {64, -12, 8}, {128, 4, -24}, {256, 4, -24} }
> +};
> +
> +/**
> + * Setting for Intel Tile-X framebuffer layout
> + */
> +static struct FBTileWalk txTileWalk = {
> +.bytesPerPixel = 4,
> +.subTileWidth = 128, .subTileHeight = 8,
> +.tileWidth = 128, .tileHeight = 8,
> +.numDirChanges = 1,
> +.dirChanges = { {8, 128, 0} }
> +};
> +
> +/**
> + * Setting for Intel Tile-Y framebuffer layout
> + * Even thou a simple generic detiling logic doesnt require the
> + * dummy 256 posOffset entry. The pseudo parallel detiling based
> + * opti logic requires to know about the Tile boundry.
> + */
> +static struct FBTileWalk tyTileWalk = {
> +.bytesPerPixel = 4,
> +.subTileWidth = 4, .subTileHeight = 32,
> +.tileWidth = 32, .tileHeight = 32,
> +.numDirChanges = 2,
> +.dirChanges = { {32, 4, 0}, {256, 4, 0} }
> +};
> +
> +
> +/**
> + * Generic Logic to Tile/Detile between tiled and linear layout.
> + *
> + * @param op whether to tile or detile
> + * @param w width of the image
> + * @param h height of the image
> + * @param dst the des

Re: [FFmpeg-devel] [PATCH 2/2] SpeedHQ encoder

2020-07-13 Thread Derek Buitenhuis
On 13/07/2020 12:31, Jean-Baptiste Kempf wrote:
> +
> +av_cold int ff_speedhq_encode_init(MpegEncContext *s)
> +{

Most of the stuff in this init function is global, and thus should be
done via ff_thread_once, and caps_internal should have 
FF_CODEC_CAP_INIT_THREADSAFE.

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

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

Re: [FFmpeg-devel] [PATCH] [PATCH] POWER8 VSX vectorization libswscale/input.c Track ticket 5570

2020-07-13 Thread Pestov Vyacheslav
Please, check my patch. It has been in pending status for a long time. I can’t 
get a bounty. If something needs to be finalized, just tell me.

https://patchwork.ffmpeg.org/project/ffmpeg/patch/1585056463-7934-1-git-send-email-pestov.vy...@yandex.ru/

https://trac.ffmpeg.org/ticket/5570

On 25.03.2020 10:55, Pestov Vyacheslav wrote:

> On 25.03.2020 1:12, Carl Eugen Hoyos wrote:
>
>> Am Di., 24. März 2020 um 14:28 Uhr schrieb Pestov Vyacheslav
>> :
>>> yuy2ToY_c: 10157
>>> yuy2ToY_c_vsx: 2353
>>>
>>> yuy2ToUV_c: 4907
>>> yuy2ToUV_c_vsx: 1357
>>>
>>> rgb24ToY_c: 21172
>>> rgb24ToY_c_vsx: 9191
>>>
>>> rgb24ToUV_c: 33568
>>> rgb24ToUV_c_vsx: 12746
>>>
>>> bgr24ToY_c: 20983
>>> bgr24ToY_c_vsx: 9381
>>>
>>> bgr24ToUV_c: 34513
>>> bgr24ToUV_c_vsx: 12708
>>>
>>> monowhite2Y_c: 5247
>>> monowhite2Y_c_vsx: 2099
>>>
>>> monoblack2Y_c: 5584
>>> monoblack2Y_c_vsx: 1993
>>>
>>> uyvyToY_c: 10111
>>> uyvyToY_c_vsx: 1780
>>>
>>> uyvyToUV_c: 4872
>>> uyvyToUV_c_vsx: 1284
>>>
>>> nvXXtoUV_c: 5128
>>> nvXXtoUV_c_vsx: 1456
>>>
>>> rgbaToA_c: 9930
>>> rgbaToA_c_vsx: 2599
>>>
>>> bswap16Y_c: 10399
>>> bswap16Y_c_vsx: 2451
>>>
>>> rgb16_32ToUV_half_c_template: 42350
>>> rgb16_32ToUV_half_c_template_vsx: 18583
>>>
>>> bswap16UV_c: 11784
>>> bswap16UV_c_vsx: 2873
>>>
>>> planar_rgb_to_y: 24602
>>> planar_rgb_to_y_vsx: 10792
>>>
>>> planar_rgb_to_uv: 35601
>>> planar_rgb_to_uv_vsx: 14112
>>>
>>> planar_rgb16_to_y: 25686
>>> planar_rgb16_to_y_vsx: 10293
>>>
>>> planar_rgb16_to_uv: 36367
>>> planar_rgb16_to_uv_vsx: 13575
>>>
>>> yvy2ToUV_c: 4879
>>> yvy2ToUV_c_vsx: 1239
>>>
>>> read_ya16be_gray_c: 9591
>>> read_ya16be_gray_c_vsx: 4164
>>>
>>> read_ya16be_alpha_c: 9390
>>> read_ya16be_alpha_c_vsx: 1874
>>>
>>> read_ya16le_gray_c: 9884
>>> read_ya16le_gray_c_vsx: 4224
>>>
>>> read_ya16le_alpha_c: 9403
>>> read_ya16le_alpha_c_vsx: 2026
>>>
>>> planar_rgb_to_a: 10262
>>> planar_rgb_to_a_vsx: 9361
>>>
>>> planar_rgb16_to_a: 9554
>>> planar_rgb16_to_a_vsx: 9393
>>>
>>> read_ayuv64le_Y_c: 10457
>>> read_ayuv64le_Y_c_vsx: 7703
>>>
>>> read_ayuv64le_A_c: 9404
>>> read_ayuv64le_A_c_vsx: 2797
>>>
>>> read_ayuv64le_UV_c: 9464
>>> read_ayuv64le_UV_c_vsx: 3781
>>>
>>> p010LEToY_c: 9546
>>> p010LEToY_c_vsx: 2422
>>>
>>> p010LEToUV_c: 6390
>>> p010LEToUV_c_vsx: 2681
>>>
>>> p010BEToY_c: 9836
>>> p010BEToY_c_vsx: 2572
>>>
>>> p010BEToUV_c: 7022
>>> p010BEToUV_c_vsx: 2660
>>>
>>> p016LEToUV_c: 5022
>>> p016LEToUV_c_vsx: 2447
>>>
>>> p016BEToUV_c: 5293
>>> p016BEToUV_c_vsx: 2307
>> To make our lives a little easier, could you tell us what you tested
>> and how we can reproduce your results?
>>
>> Also: Is your patch expected to be bit-exact? If yes, do you
>> have a script that allows to compare C and vsx code?
>> If not, how did you test your code?
>> (Or does fate cover these conversions? I wouldn't expect so.)
>>
>> Thank you, Carl Eugen
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
> Hi, yes I am using some scripts(see attached archive):
>
> 1) Put macros into tested functions in input_vsx.c and input.c   (START_TIMER 
> and STOP_TIMET). 
> You can take files with macros from my dropbox 
> https://www.dropbox.com/sh/eoed5pjp9a9psx0/AACpsa6PKGAIl5pYF58sHeRda?dl=0
>
> 2) I am generating some random input file with dd utility. For example:
>
> dd if=/dev/urandom of=/tmp/test.raw bs=1024 count=40900
>
> 3) Run script script ffmpeg_bench.sh from directory where created test.raw
>
> cd /tmp
>
> bash ./ffmpeg_bench.sh
>
> 4) Run python3 script print_result.py.  This script takes the ffmpeg_bench 
> results from files bench_cpu.txt and bench_vsx.txt, calculates the average 
> values and displays in a convenient form 
>
> With best regards, Pestov Vyacheslav
>
> ___
> 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".

Please, check my patch. It has been in pending status for a long time. I can’t 
get a bounty. If something needs to be finalized, just tell me.

-- 
With best regards, Vyacheslav Pestov

<>
___
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 v8 3/3] avdevice/decklink_dec: export timecode with s12m side data

2020-07-13 Thread Devin Heitmueller
On Sun, Jul 12, 2020 at 6:16 AM  wrote:
> I have add fate timecode testing for h264/hevc and haven't submit yet. But if
> the frame rate > 30, I got one unexpected result after map SMPTE ST 12-1:2014
> side data to HEVC timecode, the frame is 6bit only(2bit for tens of frame),
> so to framerate > 30, it'll be divided by 2 with same timecode, but HEVC 
> timecode
> frame number can use 9bit and expect the frame > 30.

You should still divide it by 2, even though HEVC supports more bits
for the frame number.  This is to be consistent with SMPTE 12-1
timecode, and downstream muxers/outputs aren't going to know whether
the upstream decoder was H.264 or HEVC.  On the encode side, you can
multiple by 2 and use the field bit to determine whether to then add
1.  See SMPTE ST 12-1:2014 Sec 12.2.

There's an argument that we should add support for higher framerates
to support SMPTE 12-3 (which supports fps > 60), but that should be a
different side data field.  Right now the most important thing is that
the behavior be consistent across decoders.

Devin

-- 
Devin Heitmueller, Senior Software Engineer
LTN Global Communications
o: +1 (301) 363-1001
w: https://ltnglobal.com  e: devin.heitmuel...@ltnglobal.com
___
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] avcodec/v4l2_m2m_dec: Remove redundant packet and fix double free

2020-07-13 Thread James Almer
On 7/12/2020 8:11 PM, Andriy Gelman wrote:
> On Wed, 24. Jun 09:26, Andriy Gelman wrote:
>> On Mon, 25. May 14:59, Andriy Gelman wrote:
>>> On Sat, 16. May 11:53, Andriy Gelman wrote:
 On Sat, 09. May 13:35, Andriy Gelman wrote:
> From: Andriy Gelman 
>
> v4l2_receive_frame() uses two packets s->buf_pkt and avpkt. If avpkt
> cannot be enqueued, the packet is buffered in s->buf_pkt and enqueued in
> the next call. Currently the ownership transfer between the two packets
> is not properly handled. A double free occurs if
> ff_v4l2_context_enqueue_packet() returns EAGAIN and v4l2_try_start
> returns EINVAL.
>
> In fact, having two AVPackets is not needed and everything can be
> handled by s->buf_pkt.
>
> This commit removes the local avpkt from v4l2_receive_frame(), meaning
> that the ownership transfer doesn't need to be handled and the double
> free is fixed.
>
> Signed-off-by: Andriy Gelman 
> ---
>
> Sorry, forgot to squash the commit from v1 so v2 didn't apply. This is 
> correct version.
>
> Supersedes:
> https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200505055454.28683-1-andriy.gel...@gmail.com/
>
>
>  libavcodec/v4l2_m2m_dec.c | 37 +++--
>  1 file changed, 15 insertions(+), 22 deletions(-)
>
> diff --git a/libavcodec/v4l2_m2m_dec.c b/libavcodec/v4l2_m2m_dec.c
> index 3e17e0fcac..b038efed9c 100644
> --- a/libavcodec/v4l2_m2m_dec.c
> +++ b/libavcodec/v4l2_m2m_dec.c
> @@ -138,14 +138,10 @@ static int v4l2_receive_frame(AVCodecContext 
> *avctx, AVFrame *frame)
>  V4L2m2mContext *s = ((V4L2m2mPriv*)avctx->priv_data)->context;
>  V4L2Context *const capture = &s->capture;
>  V4L2Context *const output = &s->output;
> -AVPacket avpkt = {0};
>  int ret;
>  
> -if (s->buf_pkt.size) {
> -avpkt = s->buf_pkt;
> -memset(&s->buf_pkt, 0, sizeof(AVPacket));
> -} else {
> -ret = ff_decode_get_packet(avctx, &avpkt);
> +if (!s->buf_pkt.size) {
> +ret = ff_decode_get_packet(avctx, &s->buf_pkt);
>  if (ret < 0 && ret != AVERROR_EOF)
>  return ret;
>  }
> @@ -153,32 +149,29 @@ static int v4l2_receive_frame(AVCodecContext 
> *avctx, AVFrame *frame)
>  if (s->draining)
>  goto dequeue;
>  
> -ret = ff_v4l2_context_enqueue_packet(output, &avpkt);
> -if (ret < 0) {
> -if (ret != AVERROR(EAGAIN))
> -   return ret;
> +ret = ff_v4l2_context_enqueue_packet(output, &s->buf_pkt);
> +if (ret < 0 && ret != AVERROR(EAGAIN))
> +goto fail;
>  
> -s->buf_pkt = avpkt;
> -/* no input buffers available, continue dequeing */
> -}
> +/* if EAGAIN don't unref packet and try to enqueue in the next 
> iteration */
> +if (ret != AVERROR(EAGAIN))
> +av_packet_unref(&s->buf_pkt);
>  
> -if (avpkt.size) {
> +if (!s->draining) {
>  ret = v4l2_try_start(avctx);
>  if (ret) {
> -av_packet_unref(&avpkt);
> -
>  /* cant recover */
> -if (ret == AVERROR(ENOMEM))
> -return ret;
> -
> -return 0;
> +if (ret != AVERROR(ENOMEM))
> +ret = 0;
> +goto fail;
>  }
>  }
>  
>  dequeue:
> -if (!s->buf_pkt.size)
> -av_packet_unref(&avpkt);
>  return ff_v4l2_context_dequeue_frame(capture, frame, -1);
> +fail:
> +av_packet_unref(&s->buf_pkt);
> +return ret;
>  }
>  
>  static av_cold int v4l2_decode_init(AVCodecContext *avctx)
> -- 
> 2.25.1
>

 ping

>>>
>>> ping
>>>
>>
>> ping
>>
> 
> The patch has been on the ml for a long time so I'd like to apply it soon.
> It gets rid of a seg fault in:
> https://trac.ffmpeg.org/ticket/8774 
> 
> Any objections?

If you maintain this code and get no reviews, then you can push it after
a while. No need to wait two months.

I can't test it, but should be ok.
___
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] x86/yuv2rgb: fix crashes when storing data on unaligned buffers

2020-07-13 Thread Ronald S. Bultje
On Mon, Jul 13, 2020 at 3:10 AM Carl Eugen Hoyos  wrote:

>
>
> > Am 13.07.2020 um 02:33 schrieb James Almer :
> >
> > Regression since fc6a5883d6af8cae0e96af84dda0ad74b360a084 on SSSE3
> enabled
> > CPUs.
>
> FFmpeg does not require aligned output buffers on x86_64?


Not in swscale/avfilter, only in avcodec.

Ronald
___
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 v3 1/4] libavutil/imgutils: add utility to get plane sizes

2020-07-13 Thread Brian Kim
This utility helps avoid undefined behavior when doing things like
checking how much memory we need to allocate for an image before we have
allocated a buffer.

Signed-off-by: Brian Kim 
---
 doc/APIchanges   |  3 ++
 libavutil/imgutils.c | 98 +---
 libavutil/imgutils.h | 11 +
 libavutil/version.h  |  2 +-
 4 files changed, 90 insertions(+), 24 deletions(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index 1d6cc36b8c..44defd9ca8 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,9 @@ libavutil: 2017-10-21
 
 API changes, most recent first:
 
+2020-07-xx - xx - lavu 56.56.100 - imgutils.h
+  Add av_image_fill_plane_sizes().
+
 2020-06-12 - b09fb030c1 - lavu 56.55.100 - pixdesc.h
   Add AV_PIX_FMT_X2RGB10.
 
diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c
index 7f9c1b632c..345b7fa94c 100644
--- a/libavutil/imgutils.c
+++ b/libavutil/imgutils.c
@@ -108,45 +108,69 @@ int av_image_fill_linesizes(int linesizes[4], enum 
AVPixelFormat pix_fmt, int wi
 return 0;
 }
 
-int av_image_fill_pointers(uint8_t *data[4], enum AVPixelFormat pix_fmt, int 
height,
-   uint8_t *ptr, const int linesizes[4])
+int av_image_fill_plane_sizes(size_t sizes[4], enum AVPixelFormat pix_fmt,
+  int height, const ptrdiff_t linesizes[4])
 {
-int i, total_size, size[4] = { 0 }, has_plane[4] = { 0 };
+int i, has_plane[4] = { 0 };
 
 const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pix_fmt);
-memset(data , 0, sizeof(data[0])*4);
+memset(sizes, 0, sizeof(sizes[0])*4);
 
 if (!desc || desc->flags & AV_PIX_FMT_FLAG_HWACCEL)
 return AVERROR(EINVAL);
 
-data[0] = ptr;
-if (linesizes[0] > (INT_MAX - 1024) / height)
+if (linesizes[0] > SIZE_MAX / height)
 return AVERROR(EINVAL);
-size[0] = linesizes[0] * height;
+sizes[0] = linesizes[0] * height;
 
 if (desc->flags & AV_PIX_FMT_FLAG_PAL ||
 desc->flags & FF_PSEUDOPAL) {
-data[1] = ptr + size[0]; /* palette is stored here as 256 32 bits 
words */
-return size[0] + 256 * 4;
+sizes[1] = 256 * 4; /* palette is stored here as 256 32 bits words */
+return 0;
 }
 
 for (i = 0; i < 4; i++)
 has_plane[desc->comp[i].plane] = 1;
 
-total_size = size[0];
 for (i = 1; i < 4 && has_plane[i]; i++) {
 int h, s = (i == 1 || i == 2) ? desc->log2_chroma_h : 0;
-data[i] = data[i-1] + size[i-1];
 h = (height + (1 << s) - 1) >> s;
-if (linesizes[i] > INT_MAX / h)
+if (linesizes[i] > SIZE_MAX / h)
 return AVERROR(EINVAL);
-size[i] = h * linesizes[i];
-if (total_size > INT_MAX - size[i])
+sizes[i] = h * linesizes[i];
+}
+
+return 0;
+}
+
+int av_image_fill_pointers(uint8_t *data[4], enum AVPixelFormat pix_fmt, int 
height,
+   uint8_t *ptr, const int linesizes[4])
+{
+int i, ret;
+ptrdiff_t linesizes1[4];
+size_t sizes[4];
+
+for (i = 0; i < 4; i++)
+linesizes1[i] = linesizes[i];
+
+ret = av_image_fill_plane_sizes(sizes, pix_fmt, height, linesizes1);
+if (ret < 0)
+return ret;
+
+ret = 0;
+for (i = 0; i < 4; i++) {
+if (sizes[i] > INT_MAX - ret)
 return AVERROR(EINVAL);
-total_size += size[i];
+ret += sizes[i];
 }
 
-return total_size;
+memset(data , 0, sizeof(data[0])*4);
+
+data[0] = ptr;
+for (i = 1; i < 4 && sizes[i - 1] > 0; i++)
+data[i] = data[i - 1] + sizes[i - 1];
+
+return ret;
 }
 
 int avpriv_set_systematic_pal2(uint32_t pal[256], enum AVPixelFormat pix_fmt)
@@ -194,6 +218,8 @@ int av_image_alloc(uint8_t *pointers[4], int linesizes[4],
 {
 const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pix_fmt);
 int i, ret;
+ptrdiff_t linesizes1[4];
+size_t total_size, sizes[4];
 uint8_t *buf;
 
 if (!desc)
@@ -204,12 +230,20 @@ int av_image_alloc(uint8_t *pointers[4], int linesizes[4],
 if ((ret = av_image_fill_linesizes(linesizes, pix_fmt, align>7 ? 
FFALIGN(w, 8) : w)) < 0)
 return ret;
 
-for (i = 0; i < 4; i++)
+for (i = 0; i < 4; i++) {
 linesizes[i] = FFALIGN(linesizes[i], align);
+linesizes1[i] = linesizes[i];
+}
 
-if ((ret = av_image_fill_pointers(pointers, pix_fmt, h, NULL, linesizes)) 
< 0)
+if ((ret = av_image_fill_plane_sizes(sizes, pix_fmt, h, linesizes1)) < 0)
 return ret;
-buf = av_malloc(ret + align);
+total_size = align;
+for (i = 0; i < 4; i++) {
+if (total_size > SIZE_MAX - sizes[i])
+return AVERROR(EINVAL);
+total_size += sizes[i];
+}
+buf = av_malloc(total_size);
 if (!buf)
 return AVERROR(ENOMEM);
 if ((ret = av_image_fill_pointers(pointers, pix_fmt, h, buf, linesizes)) < 
0) {
@@ -220,6 +254,7 @@ int av_image_alloc(uint8_t *pointers[4], int linesizes[4],

[FFmpeg-devel] [PATCH v3 2/4] libavutil/frame: avoid UB when getting plane sizes

2020-07-13 Thread Brian Kim
This uses av_image_fill_plane_sizes instead of av_image_fill_pointers
when we are getting plane sizes to avoid UB from adding offsets to NULL.

Signed-off-by: Brian Kim 
---
 libavutil/frame.c | 20 
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/libavutil/frame.c b/libavutil/frame.c
index 9884eae054..3ab1aa3242 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -212,8 +212,10 @@ void av_frame_free(AVFrame **frame)
 static int get_video_buffer(AVFrame *frame, int align)
 {
 const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(frame->format);
-int ret, i, padded_height;
+int ret, i, padded_height, total_size;
 int plane_padding = FFMAX(16 + 16/*STRIDE_ALIGN*/, align);
+ptrdiff_t linesizes[4];
+size_t sizes[4];
 
 if (!desc)
 return AVERROR(EINVAL);
@@ -238,12 +240,22 @@ static int get_video_buffer(AVFrame *frame, int align)
 frame->linesize[i] = FFALIGN(frame->linesize[i], align);
 }
 
+for (i = 0; i < 4; i++)
+linesizes[i] = frame->linesize[i];
+
 padded_height = FFALIGN(frame->height, 32);
-if ((ret = av_image_fill_pointers(frame->data, frame->format, 
padded_height,
-  NULL, frame->linesize)) < 0)
+if ((ret = av_image_fill_plane_sizes(sizes, frame->format,
+ padded_height, linesizes)) < 0)
 return ret;
 
-frame->buf[0] = av_buffer_alloc(ret + 4*plane_padding);
+total_size = 4*plane_padding;
+for (i = 0; i < 4; i++) {
+if (sizes[i] > INT_MAX - total_size)
+return AVERROR(EINVAL);
+total_size += sizes[i];
+}
+
+frame->buf[0] = av_buffer_alloc(total_size);
 if (!frame->buf[0]) {
 ret = AVERROR(ENOMEM);
 goto fail;
-- 
2.27.0.389.gc38d7665816-goog

___
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 v3 4/4] libavutil/imgutils: check for non-null buffer in av_image_fill_pointers

2020-07-13 Thread Brian Kim
We were previously always filling data by adding offsets to ptr, which
was undefined behavior when ptr was NULL.

Signed-off-by: Brian Kim 
---
 libavutil/imgutils.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c
index 345b7fa94c..721dc2784a 100644
--- a/libavutil/imgutils.c
+++ b/libavutil/imgutils.c
@@ -166,6 +166,9 @@ int av_image_fill_pointers(uint8_t *data[4], enum 
AVPixelFormat pix_fmt, int hei
 
 memset(data , 0, sizeof(data[0])*4);
 
+if (!ptr)
+return ret;
+
 data[0] = ptr;
 for (i = 1; i < 4 && sizes[i - 1] > 0; i++)
 data[i] = data[i - 1] + sizes[i - 1];
-- 
2.27.0.389.gc38d7665816-goog

___
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 v3 3/4] libavcodec/decode: avoid UB when getting plane sizes

2020-07-13 Thread Brian Kim
This uses av_image_fill_plane_sizes instead of av_image_fill_pointers
when we are getting plane sizes to avoid UB from adding offsets to NULL.

Signed-off-by: Brian Kim 
---
 libavcodec/decode.c | 27 +--
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index de9c079f9d..8ea9293ebf 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1471,12 +1471,12 @@ static int update_frame_pool(AVCodecContext *avctx, 
AVFrame *frame)
 
 switch (avctx->codec_type) {
 case AVMEDIA_TYPE_VIDEO: {
-uint8_t *data[4];
 int linesize[4];
-int size[4] = { 0 };
 int w = frame->width;
 int h = frame->height;
-int tmpsize, unaligned;
+int unaligned;
+ptrdiff_t tmpsize, linesize1[4];
+size_t sizes[4];
 
 avcodec_align_dimensions2(avctx, &w, &h, pool->stride_align);
 
@@ -1494,21 +1494,20 @@ static int update_frame_pool(AVCodecContext *avctx, 
AVFrame *frame)
 unaligned |= linesize[i] % pool->stride_align[i];
 } while (unaligned);
 
-tmpsize = av_image_fill_pointers(data, avctx->pix_fmt, h,
- NULL, linesize);
-if (tmpsize < 0) {
-ret = tmpsize;
+for (i = 0; i < 4; i++)
+linesize1[i] = linesize[i];
+ret = av_image_fill_plane_sizes(sizes, avctx->pix_fmt, h, linesize1);
+if (ret < 0)
 goto fail;
-}
-
-for (i = 0; i < 3 && data[i + 1]; i++)
-size[i] = data[i + 1] - data[i];
-size[i] = tmpsize - (data[i] - data[0]);
 
 for (i = 0; i < 4; i++) {
 pool->linesize[i] = linesize[i];
-if (size[i]) {
-pool->pools[i] = av_buffer_pool_init(size[i] + 16 + 
STRIDE_ALIGN - 1,
+if (sizes[i]) {
+if (sizes[i] > INT_MAX - (16 + STRIDE_ALIGN - 1)) {
+ret = AVERROR(EINVAL);
+goto fail;
+}
+pool->pools[i] = av_buffer_pool_init(sizes[i] + 16 + 
STRIDE_ALIGN - 1,
  CONFIG_MEMORY_POISONING ?
 NULL :
 av_buffer_allocz);
-- 
2.27.0.389.gc38d7665816-goog

___
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/4] libavcodec/jpeg2000dec : Prevent overriding SOP marker bit

2020-07-13 Thread gautamramk
From: Gautam Ramakrishnan 

Currently, the COC marker overrides the SOP marker bit.
However, only the COD marker may set this value. This
patch fixes this bug.
---
 libavcodec/jpeg2000dec.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
index 18a933077e..48ca1c37a5 100644
--- a/libavcodec/jpeg2000dec.c
+++ b/libavcodec/jpeg2000dec.c
@@ -588,7 +588,7 @@ static int get_coc(Jpeg2000DecoderContext *s, 
Jpeg2000CodingStyle *c,
uint8_t *properties)
 {
 int compno, ret;
-uint8_t has_eph;
+uint8_t has_eph, has_sop;
 
 if (bytestream2_get_bytes_left(&s->g) < 2) {
 av_log(s->avctx, AV_LOG_ERROR, "Insufficient space for COC\n");
@@ -606,8 +606,10 @@ static int get_coc(Jpeg2000DecoderContext *s, 
Jpeg2000CodingStyle *c,
 
 c  += compno;
 has_eph = c->csty & JPEG2000_CSTY_EPH;
+has_sop = c->csty & JPEG2000_CSTY_SOP;
 c->csty = bytestream2_get_byteu(&s->g);
 c->csty |= has_eph; //do not override eph present bits from COD
+c->csty |= has_sop; //do not override sop present bits from COD
 
 if ((ret = get_cox(s, c)) < 0)
 return ret;
-- 
2.17.1

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

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

[FFmpeg-devel] [PATCH 4/4] libavcodec/jpeg2000dec: Support for PPM marker

2020-07-13 Thread gautamramk
From: Gautam Ramakrishnan 

This patch adds support for PPM marker for JPEG2000
decoder. It allows the samples p1_03.j2k and p1_05.j2k
to be decoded.
---
 libavcodec/jpeg2000dec.c | 107 +++
 1 file changed, 97 insertions(+), 10 deletions(-)

diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
index 5ea6fd0b9a..f6200a81ed 100644
--- a/libavcodec/jpeg2000dec.c
+++ b/libavcodec/jpeg2000dec.c
@@ -71,6 +71,7 @@ typedef struct Jpeg2000POC {
 typedef struct Jpeg2000TilePart {
 uint8_t tile_index; // Tile index who refers the tile-part
 const uint8_t *tp_end;
+GetByteContext header_tpg;  // bit stream of header if PPM header 
is used
 GetByteContext tpg; // bit stream in tile-part
 } Jpeg2000TilePart;
 
@@ -102,6 +103,13 @@ typedef struct Jpeg2000DecoderContext {
 uint8_t cbps[4];// bits per sample in particular components
 uint8_t sgnd[4];// if a component is signed
 uint8_t properties[4];
+
+uint8_t has_ppm;
+uint8_t *packed_headers; // contains packed headers. Used only 
along with PPM marker
+int packed_headers_size;
+GetByteContext  packed_headers_stream;
+uint8_t in_tile_headers;
+
 int cdx[4], cdy[4];
 int precision;
 int ncomponents;
@@ -928,6 +936,31 @@ static int get_plt(Jpeg2000DecoderContext *s, int n)
 return 0;
 }
 
+static int get_ppm(Jpeg2000DecoderContext *s, int n)
+{
+void *new;
+
+if (n < 3) {
+av_log(s->avctx, AV_LOG_ERROR, "Invalid length for PPM data.\n");
+return AVERROR_INVALIDDATA;
+}
+s->has_ppm = 1;
+bytestream2_get_byte(&s->g); //Zppm is skipped and not used
+new = av_realloc(s->packed_headers,
+ s->packed_headers_size + n - 3);
+if (new) {
+s->packed_headers = new;
+} else
+return AVERROR(ENOMEM);
+memset(&s->packed_headers_stream, 0, sizeof(s->packed_headers_stream));
+memcpy(s->packed_headers + s->packed_headers_size,
+   s->g.buffer, n - 3);
+s->packed_headers_size += n - 3;
+bytestream2_skip(&s->g, n - 3);
+
+return 0;
+}
+
 static int get_ppt(Jpeg2000DecoderContext *s, int n)
 {
 Jpeg2000Tile *tile;
@@ -1039,8 +1072,19 @@ static int getlblockinc(Jpeg2000DecoderContext *s)
 return res;
 }
 
-static inline void select_stream(Jpeg2000DecoderContext *s, Jpeg2000Tile *tile,
+static inline void select_header(Jpeg2000DecoderContext *s, Jpeg2000Tile *tile,
  int *tp_index)
+{
+s->g = tile->tile_part[*tp_index].header_tpg;
+if (bytestream2_get_bytes_left(&s->g) == 0 && s->bit_index == 8) {
+if (*tp_index < FF_ARRAY_ELEMS(tile->tile_part) - 1) {
+s->g = tile->tile_part[++(*tp_index)].tpg;
+}
+}
+}
+
+static inline void select_stream(Jpeg2000DecoderContext *s, Jpeg2000Tile *tile,
+ int *tp_index, Jpeg2000CodingStyle *codsty)
 {
 s->g = tile->tile_part[*tp_index].tpg;
 if (bytestream2_get_bytes_left(&s->g) == 0 && s->bit_index == 8) {
@@ -1048,8 +1092,12 @@ static inline void select_stream(Jpeg2000DecoderContext 
*s, Jpeg2000Tile *tile,
 s->g = tile->tile_part[++(*tp_index)].tpg;
 }
 }
-if (bytestream2_peek_be32(&s->g) == JPEG2000_SOP_FIXED_BYTES)
-bytestream2_skip(&s->g, JPEG2000_SOP_BYTE_LENGTH);
+if (codsty->csty & JPEG2000_CSTY_SOP) {
+if (bytestream2_peek_be32(&s->g) == JPEG2000_SOP_FIXED_BYTES)
+bytestream2_skip(&s->g, JPEG2000_SOP_BYTE_LENGTH);
+else
+av_log(s->avctx, AV_LOG_ERROR, "SOP marker not found. instead 
%X\n", bytestream2_peek_be32(&s->g));
+}
 }
 
 static int jpeg2000_decode_packet(Jpeg2000DecoderContext *s, Jpeg2000Tile 
*tile, int *tp_index,
@@ -1064,10 +1112,12 @@ static int 
jpeg2000_decode_packet(Jpeg2000DecoderContext *s, Jpeg2000Tile *tile,
 return 0;
 rlevel->band[0].prec[precno].decoded_layers = layno + 1;
 // Select stream to read from
-if (tile->has_ppt)
+if (s->has_ppm)
+select_header(s, tile, tp_index);
+else if (tile->has_ppt)
 s->g = tile->packed_headers_stream;
 else
-select_stream(s, tile, tp_index);
+select_stream(s, tile, tp_index, codsty);
 
 if (!(ret = get_bits(s, 1))) {
 jpeg2000_flush(s);
@@ -1178,9 +1228,12 @@ static int jpeg2000_decode_packet(Jpeg2000DecoderContext 
*s, Jpeg2000Tile *tile,
 }
 
 // Save state of stream
-if (tile->has_ppt) {
+if (s->has_ppm) {
+tile->tile_part[*tp_index].header_tpg = s->g;
+select_stream(s, tile, tp_index, codsty);
+} else if (tile->has_ppt) {
 tile->packed_headers_stream = s->g;
-select_stream(s, tile, tp_index);
+select_stream(s, tile, tp_index, codsty);
 }
 for (bandno = 0; bandno < rlevel->nbands; bandno++) {

[FFmpeg-devel] [PATCH 2/4] libavcodec/jpeg2000 Fix PCRL Progression Order check

2020-07-13 Thread gautamramk
From: Gautam Ramakrishnan 

The PCRL progression checks were incomplete. This patch
modifes completes the check. Tested on p1_05.j2k.
---
 libavcodec/jpeg2000dec.c | 23 +++
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
index 48ca1c37a5..adaac192e9 100644
--- a/libavcodec/jpeg2000dec.c
+++ b/libavcodec/jpeg2000dec.c
@@ -1465,23 +1465,30 @@ static int 
jpeg2000_decode_packets_po_iteration(Jpeg2000DecoderContext *s, Jpeg2
 Jpeg2000Component *comp = tile->comp + compno;
 Jpeg2000CodingStyle *codsty = tile->codsty + compno;
 Jpeg2000QuantStyle *qntsty  = tile->qntsty + compno;
-int xc = x / s->cdx[compno];
-int yc = y / s->cdy[compno];
+
+if (!s->cdx[compno] || !s->cdy[compno])
+return AVERROR_INVALIDDATA;
 
 for (reslevelno = RSpoc; reslevelno < 
FFMIN(codsty->nreslevels, REpoc); reslevelno++) {
 unsigned prcx, prcy;
 uint8_t reducedresno = codsty->nreslevels - 1 
-reslevelno; //  ==> N_L - r
 Jpeg2000ResLevel *rlevel = comp->reslevel + reslevelno;
+int trx0, try0;
 
-if (yc % (1LL << (rlevel->log2_prec_height + 
reducedresno)) && y != tile->coord[1][0]) //FIXME this is a subset of the check
-continue;
+trx0 = ff_jpeg2000_ceildiv(tile->coord[0][0], 
s->cdx[compno] << reducedresno);
+try0 = ff_jpeg2000_ceildiv(tile->coord[1][0], 
s->cdy[compno] << reducedresno);
 
-if (xc % (1LL << (rlevel->log2_prec_width + 
reducedresno)) && x != tile->coord[0][0]) //FIXME this is a subset of the check
-continue;
+if (!(y % ((uint64_t)s->cdy[compno] << 
(rlevel->log2_prec_height + reducedresno)) == 0 ||
+ (y == tile->coord[1][0] && (try0 << reducedresno) 
% (1U << (reducedresno + rlevel->log2_prec_height)
+ continue;
+
+if (!(x % ((uint64_t)s->cdx[compno] << 
(rlevel->log2_prec_width + reducedresno)) == 0 ||
+ (x == tile->coord[0][0] && (trx0 << reducedresno) 
% (1U << (reducedresno + rlevel->log2_prec_width)
+ continue;
 
 // check if a precinct exists
-prcx   = ff_jpeg2000_ceildivpow2(xc, reducedresno) >> 
rlevel->log2_prec_width;
-prcy   = ff_jpeg2000_ceildivpow2(yc, reducedresno) >> 
rlevel->log2_prec_height;
+prcx   = ff_jpeg2000_ceildiv(x, s->cdx[compno] << 
reducedresno) >> rlevel->log2_prec_width;
+prcy   = ff_jpeg2000_ceildiv(y, s->cdy[compno] << 
reducedresno) >> rlevel->log2_prec_height;
 prcx  -= ff_jpeg2000_ceildivpow2(comp->coord_o[0][0], 
reducedresno) >> rlevel->log2_prec_width;
 prcy  -= ff_jpeg2000_ceildivpow2(comp->coord_o[1][0], 
reducedresno) >> rlevel->log2_prec_height;
 
-- 
2.17.1

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

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

[FFmpeg-devel] [PATCH 3/4] libavcodec/jpeg2000 Fix RPCL Progression order check

2020-07-13 Thread gautamramk
From: Gautam Ramakrishnan 

The RPCL progression order check was incomplete. This
patch completes the check. Tested on p1_07.j2k.
---
 libavcodec/jpeg2000dec.c | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
index adaac192e9..5ea6fd0b9a 100644
--- a/libavcodec/jpeg2000dec.c
+++ b/libavcodec/jpeg2000dec.c
@@ -1395,22 +1395,28 @@ static int 
jpeg2000_decode_packets_po_iteration(Jpeg2000DecoderContext *s, Jpeg2
 uint8_t reducedresno = codsty->nreslevels - 1 
-reslevelno; //  ==> N_L - r
 Jpeg2000ResLevel *rlevel = comp->reslevel + reslevelno;
 unsigned prcx, prcy;
+int trx0, try0;
 
-int xc = x / s->cdx[compno];
-int yc = y / s->cdy[compno];
+if (!s->cdx[compno] || !s->cdy[compno])
+return AVERROR_INVALIDDATA;
+
+trx0 = ff_jpeg2000_ceildiv(tile->coord[0][0], 
s->cdx[compno] << reducedresno);
+try0 = ff_jpeg2000_ceildiv(tile->coord[1][0], 
s->cdy[compno] << reducedresno);
 
 if (reslevelno >= codsty->nreslevels)
 continue;
 
-if (yc % (1LL << (rlevel->log2_prec_height + 
reducedresno)) && y != tile->coord[1][0]) //FIXME this is a subset of the check
+if (!(y % ((uint64_t)s->cdy[compno] << 
(rlevel->log2_prec_height + reducedresno)) == 0 ||
+ (y == tile->coord[1][0] && (try0 << reducedresno) 
% (1U << (reducedresno + rlevel->log2_prec_height)
 continue;
 
-if (xc % (1LL << (rlevel->log2_prec_width + 
reducedresno)) && x != tile->coord[0][0]) //FIXME this is a subset of the check
+if (!(x % ((uint64_t)s->cdx[compno] << 
(rlevel->log2_prec_width + reducedresno)) == 0 ||
+ (x == tile->coord[0][0] && (trx0 << reducedresno) 
% (1U << (reducedresno + rlevel->log2_prec_width)
 continue;
 
 // check if a precinct exists
-prcx   = ff_jpeg2000_ceildivpow2(xc, reducedresno) >> 
rlevel->log2_prec_width;
-prcy   = ff_jpeg2000_ceildivpow2(yc, reducedresno) >> 
rlevel->log2_prec_height;
+prcx   = ff_jpeg2000_ceildiv(x, s->cdx[compno] << 
reducedresno) >> rlevel->log2_prec_width;
+prcy   = ff_jpeg2000_ceildiv(y, s->cdy[compno] << 
reducedresno) >> rlevel->log2_prec_height;
 prcx  -= ff_jpeg2000_ceildivpow2(comp->coord_o[0][0], 
reducedresno) >> rlevel->log2_prec_width;
 prcy  -= ff_jpeg2000_ceildivpow2(comp->coord_o[1][0], 
reducedresno) >> rlevel->log2_prec_height;
 
-- 
2.17.1

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

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

Re: [FFmpeg-devel] [PATCH v3 1/4] libavutil/imgutils: add utility to get plane sizes

2020-07-13 Thread James Almer
On 7/13/2020 2:09 PM, Brian Kim wrote:
> This utility helps avoid undefined behavior when doing things like
> checking how much memory we need to allocate for an image before we have
> allocated a buffer.
> 
> Signed-off-by: Brian Kim 
> ---
>  doc/APIchanges   |  3 ++
>  libavutil/imgutils.c | 98 +---
>  libavutil/imgutils.h | 11 +
>  libavutil/version.h  |  2 +-
>  4 files changed, 90 insertions(+), 24 deletions(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 1d6cc36b8c..44defd9ca8 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -15,6 +15,9 @@ libavutil: 2017-10-21
>  
>  API changes, most recent first:
>  
> +2020-07-xx - xx - lavu 56.56.100 - imgutils.h
> +  Add av_image_fill_plane_sizes().
> +
>  2020-06-12 - b09fb030c1 - lavu 56.55.100 - pixdesc.h
>Add AV_PIX_FMT_X2RGB10.
>  
> diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c
> index 7f9c1b632c..345b7fa94c 100644
> --- a/libavutil/imgutils.c
> +++ b/libavutil/imgutils.c
> @@ -108,45 +108,69 @@ int av_image_fill_linesizes(int linesizes[4], enum 
> AVPixelFormat pix_fmt, int wi
>  return 0;
>  }
>  
> -int av_image_fill_pointers(uint8_t *data[4], enum AVPixelFormat pix_fmt, int 
> height,
> -   uint8_t *ptr, const int linesizes[4])
> +int av_image_fill_plane_sizes(size_t sizes[4], enum AVPixelFormat pix_fmt,
> +  int height, const ptrdiff_t linesizes[4])
>  {
> -int i, total_size, size[4] = { 0 }, has_plane[4] = { 0 };
> +int i, has_plane[4] = { 0 };
>  
>  const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pix_fmt);
> -memset(data , 0, sizeof(data[0])*4);
> +memset(sizes, 0, sizeof(sizes[0])*4);
>  
>  if (!desc || desc->flags & AV_PIX_FMT_FLAG_HWACCEL)
>  return AVERROR(EINVAL);
>  
> -data[0] = ptr;
> -if (linesizes[0] > (INT_MAX - 1024) / height)
> +if (linesizes[0] > SIZE_MAX / height)
>  return AVERROR(EINVAL);
> -size[0] = linesizes[0] * height;
> +sizes[0] = linesizes[0] * height;

You would need to cast height to size_t for this, i think, but seeing
av_image_check_size() currently rejects line sizes and plane sizes
bigger than INT_MAX, maybe we should just keep INT_MAX in the above
check instead (No need to send a new revision for this, i can amend it
before pushing with either solution. I just want your opinion).

>  
>  if (desc->flags & AV_PIX_FMT_FLAG_PAL ||
>  desc->flags & FF_PSEUDOPAL) {
> -data[1] = ptr + size[0]; /* palette is stored here as 256 32 bits 
> words */
> -return size[0] + 256 * 4;
> +sizes[1] = 256 * 4; /* palette is stored here as 256 32 bits words */
> +return 0;
>  }
>  
>  for (i = 0; i < 4; i++)
>  has_plane[desc->comp[i].plane] = 1;
>  
> -total_size = size[0];
>  for (i = 1; i < 4 && has_plane[i]; i++) {
>  int h, s = (i == 1 || i == 2) ? desc->log2_chroma_h : 0;
> -data[i] = data[i-1] + size[i-1];
>  h = (height + (1 << s) - 1) >> s;
> -if (linesizes[i] > INT_MAX / h)
> +if (linesizes[i] > SIZE_MAX / h)
>  return AVERROR(EINVAL);
> -size[i] = h * linesizes[i];
> -if (total_size > INT_MAX - size[i])
> +sizes[i] = h * linesizes[i];

Same here.

Rest LGTM.
___
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] avformat/crypto.c: remove unnecessary code

2020-07-13 Thread Tomas Härdin
lör 2020-07-11 klockan 16:04 +0800 skrev Steven Liu:
> Because the newpos variable is set value before use it.
> The newpos variable declared at the head partition of crypto_seek.
> 
> Signed-off-by: Steven Liu 
> ---
>  libavformat/crypto.c | 11 ---
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/libavformat/crypto.c b/libavformat/crypto.c
> index 31f9ac0ab9..daa29ed501 100644
> --- a/libavformat/crypto.c
> +++ b/libavformat/crypto.c
> @@ -252,21 +252,18 @@ static int64_t crypto_seek(URLContext *h, int64_t pos, 
> int whence)
>  case SEEK_CUR:
>  pos = pos + c->position;
>  break;
> -case SEEK_END: {
> -int64_t newpos = ffurl_seek( c->hd, pos, AVSEEK_SIZE );
> +case SEEK_END:
> +newpos = ffurl_seek( c->hd, pos, AVSEEK_SIZE );

Make me wonder why this was declared like this in the first place.
Maybe move the definition of the newpos in the outer scope to where
it's used instead? FFmpeg is C99 now so that should be fine.

>  if (newpos < 0) {
>  av_log(h, AV_LOG_ERROR,
>  "Crypto: seek_end - can't get file size (pos=%lld)\r\n", 
> (long long int)pos);
>  return newpos;
>  }
>  pos = newpos - pos;
> -}
>  break;
> -case AVSEEK_SIZE: {
> -int64_t newpos = ffurl_seek( c->hd, pos, AVSEEK_SIZE );
> +case AVSEEK_SIZE:
> +newpos = ffurl_seek( c->hd, pos, AVSEEK_SIZE );
>  return newpos;

Why not just return ffurl_seek()?

/Tomas

___
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] x86/yuv2rgb: fix crashes when storing data on unaligned buffers

2020-07-13 Thread Michael Niedermayer
On Mon, Jul 13, 2020 at 10:19:07AM -0300, James Almer wrote:
> On 7/13/2020 4:02 AM, Carl Eugen Hoyos wrote:
> > 
> > 
> >> Am 13.07.2020 um 02:33 schrieb James Almer :
> >>
> >> Regression since fc6a5883d6af8cae0e96af84dda0ad74b360a084 on SSSE3 enabled
> >> CPUs.
> > 
> > FFmpeg does not require aligned output buffers on x86_64?
> 
> There's no alignment requirement mentioned anywhere in swscale.h, which
> explains why every other asm function also uses unaligned mov
> instructions when handling src and dst buffers.

swscale will print a warning though if its provided with unaligned
buffers or strides
So i would assume that most library users attempt to supply it with
aligned buffers.
Thats something new optimized code could take advantage of

thx

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

If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.


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/golomb: Prevent shift by negative number

2020-07-13 Thread Tomas Härdin
fre 2020-07-10 klockan 15:48 +0200 skrev Andreas Rheinhardt:
> This happened in get_ue_golomb() if the cached bitstream reader was
> in
> use, because there was no check to handle the case of the read value
> not being in the range 0..8190.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/golomb.h | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
> index 7fd46a91bd..5bfcfe085f 100644
> --- a/libavcodec/golomb.h
> +++ b/libavcodec/golomb.h
> @@ -66,6 +66,10 @@ static inline int get_ue_golomb(GetBitContext *gb)
>  return ff_ue_golomb_vlc_code[buf];
>  } else {
>  int log = 2 * av_log2(buf) - 31;
> +if (log < 0) {
> +av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
> +return AVERROR_INVALIDDATA;
> +}

How come invalid codes can even be present like this? Seems
inefficient. Or is the decoder wrong? Maybe Michael wants to chime in
here, since he wrote the original implementation.

Reading a bit about Exp-Golomb it seems counting trailing zeroes would 
be the way to go. There's even a builtin for it, __builtin_ctz().
Alternatively a table-driven approach could be used.

/Tomas

___
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/golomb: Prevent shift by negative number

2020-07-13 Thread Andreas Rheinhardt
Tomas Härdin:
> fre 2020-07-10 klockan 15:48 +0200 skrev Andreas Rheinhardt:
>> This happened in get_ue_golomb() if the cached bitstream reader was
>> in
>> use, because there was no check to handle the case of the read value
>> not being in the range 0..8190.
>>
>> Signed-off-by: Andreas Rheinhardt 
>> ---
>>  libavcodec/golomb.h | 4 
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
>> index 7fd46a91bd..5bfcfe085f 100644
>> --- a/libavcodec/golomb.h
>> +++ b/libavcodec/golomb.h
>> @@ -66,6 +66,10 @@ static inline int get_ue_golomb(GetBitContext *gb)
>>  return ff_ue_golomb_vlc_code[buf];
>>  } else {
>>  int log = 2 * av_log2(buf) - 31;
>> +if (log < 0) {
>> +av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
>> +return AVERROR_INVALIDDATA;
>> +}
> 
> How come invalid codes can even be present like this? Seems
> inefficient. Or is the decoder wrong? Maybe Michael wants to chime in
> here, since he wrote the original implementation.
> 
This function is only supposed to be used when the expected value of the
exp-golomb code is in the range 0..8190 (or other words: if there are
not more than 12 leading zeroes). And just because a specification says
that a value must be smaller than that does not mean that it is actually
fulfilled in real files.

(Btw: The cached bitstream reader version of this function can actually
handle 16 leading zeroes (and this patch only errors out if there are
more); the limit of 12 is imposed because the noncached bitstream reader
can't handle more.)

> Reading a bit about Exp-Golomb it seems counting trailing zeroes would 
> be the way to go. There's even a builtin for it, __builtin_ctz().
> Alternatively a table-driven approach could be used.
> 
Here is the code in question for the cached bitstream reader:

buf = show_bits_long(gb, 32);

if (buf >= (1 << 27)) {
buf >>= 32 - 9;
skip_bits_long(gb, ff_golomb_vlc_len[buf]);

return ff_ue_golomb_vlc_code[buf];
} else {
int log = 2 * av_log2(buf) - 31;
buf >>= log;
buf--;
skip_bits_long(gb, 32 - log);

return buf;
}

So you see we are already using a table-driven approach for small values
(namely for those values where the number of leading zeroes is <= 4) and
if we are outside of that range, an approach using av_log2 (which for
supported systems is implemented using __builtin_clz() (not trailing
zeroes!)) is used.

- 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 4/4] avformat/au: Avoid allocation for metadata string

2020-07-13 Thread Andreas Rheinhardt
When there are potentially annotation (i.e. metadata) fields to write,
au_get_annotations() is called to produce a string with them. To do so,
it uses an AVBPrint which is finalized to create the string. This is
wasteful, because it always leads to an allocation even if the string
actually fits into the internal buffer of the AVBPrint. This commit
changes this by making au_get_annotations() modify an AVBPrint that
resides on the stack of the caller (i.e. of au_write_header()).

Furthermore, the AVBPrint is now checked for truncation; limiting
the allocations implicit in the AVBPrint allowed to offload the overflow
checks. Notice that these were not correct before: The size parameter of
avio_write() is an int, yet the string in the AVBPrint was allowed to
grow bigger than INT_MAX. And if the length of the string was so near
UINT_MAX that the length + 32 overflowed, the old code would write the
first eight bytes of the string and nothing more, leading to an invalid
file.

Finally, the special case in which the metadata dictionary of the
AVFormatContext is empty (in which case one still has to write eight
binary zeroes) is now no longer treated specially, because this case
no longer incurs any allocation.

Signed-off-by: Andreas Rheinhardt 
---
 libavformat/au.c | 50 ++--
 1 file changed, 19 insertions(+), 31 deletions(-)

diff --git a/libavformat/au.c b/libavformat/au.c
index a8906a9db7..c09f4da4c9 100644
--- a/libavformat/au.c
+++ b/libavformat/au.c
@@ -35,8 +35,6 @@
 
 /* if we don't know the size in advance */
 #define AU_UNKNOWN_SIZE ((uint32_t)(~0))
-/* the specification requires an annotation field of at least eight bytes */
-#define AU_DEFAULT_HEADER_SIZE (24+8)
 
 static const AVCodecTag codec_au_tags[] = {
 { AV_CODEC_ID_PCM_MULAW,  1 },
@@ -241,7 +239,7 @@ typedef struct AUContext {
 
 #include "rawenc.h"
 
-static int au_get_annotations(AVFormatContext *s, char **buffer)
+static int au_get_annotations(AVFormatContext *s, AVBPrint *annotations)
 {
 static const char keys[][7] = {
 "Title",
@@ -253,21 +251,19 @@ static int au_get_annotations(AVFormatContext *s, char 
**buffer)
 int cnt = 0;
 AVDictionary *m = s->metadata;
 AVDictionaryEntry *t = NULL;
-AVBPrint bprint;
-
-av_bprint_init(&bprint, 64, AV_BPRINT_SIZE_UNLIMITED);
 
 for (int i = 0; i < FF_ARRAY_ELEMS(keys); i++) {
 t = av_dict_get(m, keys[i], NULL, 0);
 if (t != NULL) {
 if (cnt++)
-av_bprint_chars(&bprint, '\n', 1);
-av_bprintf(&bprint, "%s=%s", keys[i], t->value);
+av_bprint_chars(annotations, '\n', 1);
+av_bprintf(annotations, "%s=%s", keys[i], t->value);
 }
 }
-/* pad with 0's */
-av_bprint_chars(&bprint, '\0', 8);
-return av_bprint_finalize(&bprint, buffer);
+/* The specification requires the annotation field to be zero-terminated
+ * and its length to be a multiple of eight, so pad with 0's */
+av_bprint_chars(annotations, '\0', 8);
+return av_bprint_is_complete(annotations) ? 0 : AVERROR(ENOMEM);
 }
 
 static int au_write_header(AVFormatContext *s)
@@ -276,9 +272,7 @@ static int au_write_header(AVFormatContext *s)
 AUContext *au = s->priv_data;
 AVIOContext *pb = s->pb;
 AVCodecParameters *par = s->streams[0]->codecpar;
-char *annotations = NULL;
-
-au->header_size = AU_DEFAULT_HEADER_SIZE;
+AVBPrint annotations;
 
 if (s->nb_streams != 1) {
 av_log(s, AV_LOG_ERROR, "only one stream is supported\n");
@@ -291,30 +285,24 @@ static int au_write_header(AVFormatContext *s)
 return AVERROR(EINVAL);
 }
 
-if (av_dict_count(s->metadata) > 0) {
-ret = au_get_annotations(s, &annotations);
-if (ret < 0)
-return ret;
-if (annotations != NULL) {
-au->header_size = (24 + strlen(annotations) + 8) & ~7;
-if (au->header_size < AU_DEFAULT_HEADER_SIZE)
-au->header_size = AU_DEFAULT_HEADER_SIZE;
-}
-}
+av_bprint_init(&annotations, 0, INT_MAX - 24);
+ret = au_get_annotations(s, &annotations);
+if (ret < 0)
+goto fail;
+au->header_size = 24 + annotations.len & ~7;
+
 ffio_wfourcc(pb, ".snd");   /* magic number */
 avio_wb32(pb, au->header_size); /* header size */
 avio_wb32(pb, AU_UNKNOWN_SIZE); /* data size */
 avio_wb32(pb, par->codec_tag);  /* codec ID */
 avio_wb32(pb, par->sample_rate);
 avio_wb32(pb, par->channels);
-if (annotations != NULL) {
-avio_write(pb, annotations, au->header_size - 24);
-av_freep(&annotations);
-} else {
-avio_wb64(pb, 0); /* annotation field */
-}
+avio_write(pb, annotations.str, annotations.len & ~7);
 
-return 0;
+fail:
+av_bprint_finalize(&annotations, NULL);
+
+return ret;
 }
 
 static int au_write_trailer(AVFormatContext *s)

[FFmpeg-devel] [PATCH 3/4] avformat/au: Simplify writing string into AVBPrint

2020-07-13 Thread Andreas Rheinhardt
by using av_bprintf() instead of several av_bprint_append().

Signed-off-by: Andreas Rheinhardt 
---
 libavformat/au.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/libavformat/au.c b/libavformat/au.c
index c4a32ff76c..a8906a9db7 100644
--- a/libavformat/au.c
+++ b/libavformat/au.c
@@ -262,13 +262,11 @@ static int au_get_annotations(AVFormatContext *s, char 
**buffer)
 if (t != NULL) {
 if (cnt++)
 av_bprint_chars(&bprint, '\n', 1);
-av_bprint_append_data(&bprint, keys[i], strlen(keys[i]));
-av_bprint_chars(&bprint, '=', 1);
-av_bprint_append_data(&bprint, t->value, strlen(t->value));
+av_bprintf(&bprint, "%s=%s", keys[i], t->value);
 }
 }
 /* pad with 0's */
-av_bprint_append_data(&bprint, "\0\0\0\0\0\0\0\0", 8);
+av_bprint_chars(&bprint, '\0', 8);
 return av_bprint_finalize(&bprint, buffer);
 }
 
-- 
2.20.1

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

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

[FFmpeg-devel] [PATCH 2/4] avformat/au: Remove redundant av_freep()

2020-07-13 Thread Andreas Rheinhardt
This av_freep(&key) in conjunction with the fact that the loop condition
checks for key != NULL was equivalent to a av_freep(&key) + a break
immediately thereafter. But given that there is an av_freep(&key)
directly after the loop, the av_freep(&key) is unnecessary and the break
can also be added explicitly.

Signed-off-by: Andreas Rheinhardt 
---
 libavformat/au.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavformat/au.c b/libavformat/au.c
index b419c9ed95..c4a32ff76c 100644
--- a/libavformat/au.c
+++ b/libavformat/au.c
@@ -107,11 +107,11 @@ static int au_read_annotation(AVFormatContext *s, int 
size)
 av_log(s, AV_LOG_ERROR, "Memory error while parsing AU 
metadata.\n");
 } else {
 av_bprint_init(&bprint, 64, AV_BPRINT_SIZE_UNLIMITED);
-for (i = 0; i < FF_ARRAY_ELEMS(keys) && key != NULL; i++) {
+for (i = 0; i < FF_ARRAY_ELEMS(keys); i++) {
 if (av_strcasecmp(keys[i], key) == 0) {
 av_dict_set(&(s->metadata), keys[i], value, 
AV_DICT_DONT_STRDUP_VAL);
-av_freep(&key);
 value = NULL;
+break;
 }
 }
 }
-- 
2.20.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/golomb: Prevent shift by negative number

2020-07-13 Thread Michael Niedermayer
On Mon, Jul 13, 2020 at 09:04:30PM +0200, Tomas Härdin wrote:
> fre 2020-07-10 klockan 15:48 +0200 skrev Andreas Rheinhardt:
> > This happened in get_ue_golomb() if the cached bitstream reader was
> > in
> > use, because there was no check to handle the case of the read value
> > not being in the range 0..8190.
> > 
> > Signed-off-by: Andreas Rheinhardt 
> > ---
> >  libavcodec/golomb.h | 4 
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
> > index 7fd46a91bd..5bfcfe085f 100644
> > --- a/libavcodec/golomb.h
> > +++ b/libavcodec/golomb.h
> > @@ -66,6 +66,10 @@ static inline int get_ue_golomb(GetBitContext *gb)
> >  return ff_ue_golomb_vlc_code[buf];
> >  } else {
> >  int log = 2 * av_log2(buf) - 31;
> > +if (log < 0) {
> > +av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
> > +return AVERROR_INVALIDDATA;
> > +}
> 
> How come invalid codes can even be present like this? Seems
> inefficient. Or is the decoder wrong? Maybe Michael wants to chime in
> here, since he wrote the original implementation.

The codepath of the non cached one does a check too. It should be
done consistently.
If the check causes a meassurable speedloss, then it can be implemented
without a check. For example by using asm for the shift operation, or maybe
compilers have some nicer function doing a never undefined shift. If such
function does not exist, its something that maybe someone should add to
gcc & clang because other projects could benefit from it too in similar
situations.
Of course all such improvments make the code more complex as a fallback
with check is needed if none of the non standard "safe" functions exist.
So it only makes sense to go that way if this actually affects speed.
It is after all not on the main codepath which is handled with a table

thx

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

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire


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 1/4] avformat/au: Store strings instead of pointers to strings in array

2020-07-13 Thread Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt 
---
 libavformat/au.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/libavformat/au.c b/libavformat/au.c
index f92863e400..b419c9ed95 100644
--- a/libavformat/au.c
+++ b/libavformat/au.c
@@ -68,13 +68,13 @@ static int au_probe(const AVProbeData *p)
 
 static int au_read_annotation(AVFormatContext *s, int size)
 {
-static const char * keys[] = {
+static const char keys[][7] = {
 "title",
 "artist",
 "album",
 "track",
 "genre",
-NULL };
+};
 AVIOContext *pb = s->pb;
 enum { PARSE_KEY, PARSE_VALUE, PARSE_FINISHED } state = PARSE_KEY;
 char c;
@@ -107,7 +107,7 @@ static int au_read_annotation(AVFormatContext *s, int size)
 av_log(s, AV_LOG_ERROR, "Memory error while parsing AU 
metadata.\n");
 } else {
 av_bprint_init(&bprint, 64, AV_BPRINT_SIZE_UNLIMITED);
-for (i = 0; keys[i] != NULL && key != NULL; i++) {
+for (i = 0; i < FF_ARRAY_ELEMS(keys) && key != NULL; i++) {
 if (av_strcasecmp(keys[i], key) == 0) {
 av_dict_set(&(s->metadata), keys[i], value, 
AV_DICT_DONT_STRDUP_VAL);
 av_freep(&key);
@@ -243,14 +243,13 @@ typedef struct AUContext {
 
 static int au_get_annotations(AVFormatContext *s, char **buffer)
 {
-static const char * keys[] = {
+static const char keys[][7] = {
 "Title",
 "Artist",
 "Album",
 "Track",
 "Genre",
-NULL };
-int i;
+};
 int cnt = 0;
 AVDictionary *m = s->metadata;
 AVDictionaryEntry *t = NULL;
@@ -258,7 +257,7 @@ static int au_get_annotations(AVFormatContext *s, char 
**buffer)
 
 av_bprint_init(&bprint, 64, AV_BPRINT_SIZE_UNLIMITED);
 
-for (i = 0; keys[i] != NULL; i++) {
+for (int i = 0; i < FF_ARRAY_ELEMS(keys); i++) {
 t = av_dict_get(m, keys[i], NULL, 0);
 if (t != NULL) {
 if (cnt++)
-- 
2.20.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] [PATCH] POWER8 VSX vectorization libswscale/input.c Track ticket 5570

2020-07-13 Thread Michael Niedermayer
Hi

On Mon, Jul 13, 2020 at 05:38:40PM +0300, Pestov Vyacheslav wrote:
> Please, check my patch. It has been in pending status for a long time. I 
> can’t get a bounty. If something needs to be finalized, just tell me.
> 
> https://patchwork.ffmpeg.org/project/ffmpeg/patch/1585056463-7934-1-git-send-email-pestov.vy...@yandex.ru/
> 
> https://trac.ffmpeg.org/ticket/5570

There was a previous patch IIRC by Dan Parrot to which again IIRC
ronald objected due to use of intrinsics instead of hand written asm.

I did not entirely agree with ronald though i always saw his point and id
like to hear his oppinion on this patchset. Also i dont have a working ppc 
with which i could test this ATM.

So to move forward what we need is
1. ronalds oppinion
2. someone with the right hardware needs to test and confirm both
that this is faster and that it works correctly
3. simple code review

If these 3 pass, my oppinion is that its an improvment and it could be
applied.

thx

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

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


signature.asc
Description: 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] [PATCH] POWER8 VSX vectorization libswscale/input.c Track ticket 5570

2020-07-13 Thread Michael Niedermayer
On Mon, Jul 13, 2020 at 10:23:28PM +0200, Michael Niedermayer wrote:
> Hi
> 
> On Mon, Jul 13, 2020 at 05:38:40PM +0300, Pestov Vyacheslav wrote:
> > Please, check my patch. It has been in pending status for a long time. I 
> > can’t get a bounty. If something needs to be finalized, just tell me.
> > 
> > https://patchwork.ffmpeg.org/project/ffmpeg/patch/1585056463-7934-1-git-send-email-pestov.vy...@yandex.ru/
> > 
> > https://trac.ffmpeg.org/ticket/5570
> 

> There was a previous patch IIRC by Dan Parrot to which again IIRC
> ronald objected due to use of intrinsics instead of hand written asm.

to clarify this, and this is all IIRC ronald did a great and harsh review
of the previous patchset pointing at many loose ends and issues.
Thats why i do want to hear his oppinion on this patchset ...


[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- Plato


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] libavcodec/jpeg2000dec: Support for PPM marker

2020-07-13 Thread Michael Niedermayer
On Mon, Jul 13, 2020 at 10:50:02PM +0530, gautamr...@gmail.com wrote:
> From: Gautam Ramakrishnan 
> 
> This patch adds support for PPM marker for JPEG2000
> decoder. It allows the samples p1_03.j2k and p1_05.j2k
> to be decoded.
> ---
>  libavcodec/jpeg2000dec.c | 107 +++
>  1 file changed, 97 insertions(+), 10 deletions(-)
[...]

> @@ -2189,8 +2266,18 @@ static int 
> jpeg2000_read_main_headers(Jpeg2000DecoderContext *s)
>  // Packet length, tile-part header
>  ret = get_plt(s, len);
>  break;
> +case JPEG2000_PPM:
> +// Packed headers, main header
> +ret = get_ppm(s, len);
> +break;
>  case JPEG2000_PPT:
>  // Packed headers, tile-part header
> +if (s->has_ppm) {
> +av_log(s->avctx, AV_LOG_ERROR,
> +   "Cannot have both PPT and PPM marker.\n");
> +return AVERROR_INVALIDDATA;
> +}

is a similar check needed before get_ppm() ?

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

No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes


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 1/2] mpeg2: Renaming functions around init_uni_ac_vlc

2020-07-13 Thread Michael Niedermayer
On Mon, Jul 13, 2020 at 01:16:18PM +0200, Jean-Baptiste Kempf wrote:
> We need to export init_uni_ac_vlc to init_uni_ac_vlc and therefore need
> renaming ff_init_uni_ac_vlc to ff_init_uni_ac_huffman for the following
> patch.
> ---
>  libavcodec/mjpegenc.c| 4 ++--
>  libavcodec/mjpegenc_common.c | 6 +++---
>  libavcodec/mjpegenc_common.h | 2 +-
>  libavcodec/mpeg12.h  | 2 ++
>  libavcodec/mpeg12enc.c   | 6 +++---
>  libavcodec/speedhq.c | 2 +-
>  6 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/libavcodec/mjpegenc.c b/libavcodec/mjpegenc.c
> index 56ccbc5fb1..eaa739306a 100644
> --- a/libavcodec/mjpegenc.c
> +++ b/libavcodec/mjpegenc.c
> @@ -106,8 +106,8 @@ av_cold int ff_mjpeg_encode_init(MpegEncContext *s)
>   avpriv_mjpeg_bits_ac_chrominance,
>   avpriv_mjpeg_val_ac_chrominance);
>  
> -ff_init_uni_ac_vlc(m->huff_size_ac_luminance,   m->uni_ac_vlc_len);
> -ff_init_uni_ac_vlc(m->huff_size_ac_chrominance, 
> m->uni_chroma_ac_vlc_len);
> +ff_init_uni_ac_huffman(m->huff_size_ac_luminance,   m->uni_ac_vlc_len);
> +ff_init_uni_ac_huffman(m->huff_size_ac_chrominance, 
> m->uni_chroma_ac_vlc_len);
>  s->intra_ac_vlc_length  =
>  s->intra_ac_vlc_last_length = m->uni_ac_vlc_len;
>  s->intra_chroma_ac_vlc_length  =
> diff --git a/libavcodec/mjpegenc_common.c b/libavcodec/mjpegenc_common.c
> index 3038ebde6e..fed32efc67 100644
> --- a/libavcodec/mjpegenc_common.c
> +++ b/libavcodec/mjpegenc_common.c
> @@ -36,7 +36,7 @@
>  #include "mjpegenc_huffman.h"
>  #include "mjpeg.h"
>  
> -av_cold void ff_init_uni_ac_vlc(const uint8_t huff_size_ac[256], uint8_t 
> *uni_ac_vlc_len)
> +av_cold void ff_init_uni_ac_huffman(const uint8_t huff_size_ac[256], uint8_t 
> *uni_ac_vlc_len)
>  {
>  int i;
>  

Maybe its nitpicking but huffman != vlc
huffman is a specific type of vlc code
and specifically, huffman codes are optimal and have no unused parts while
this here can have unused parts so huffman may be a suboptimal term

also isnt this jpeg specific here ?
if so adding a "jpeg" in the name might work better

thx

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

If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.


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

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

Re: [FFmpeg-devel] [PATCH v2 2/3] doc/developer.texi: Restructured "Submitting patches" section.

2020-07-13 Thread Michael Niedermayer
On Sun, Jul 12, 2020 at 11:53:00PM +0200, Manolis Stamatogiannakis wrote:
> - Main text split to two sections.
> - Detailed checklist for new codecs or formats demoted to section.
> - Detailed checklist for patch submission demoted to section.

If a commit message needs a list enumerating something then often
this is a sign the patch should be split
I think this is the case here, these things are unrelated of each other

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

Into a blind darkness they enter who follow after the Ignorance,
they as if into a greater darkness enter who devote themselves
to the Knowledge alone. -- Isha Upanishad


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] libavfilter/vf_codecview: enable qp visualization for H264 and VP9

2020-07-13 Thread Yongle Lin
On Thu, Jul 9, 2020 at 2:18 PM Yongle Lin  wrote:

>
>
> On Mon, Jul 6, 2020 at 10:56 AM Yongle Lin 
> wrote:
>
>>
>>
>> On Thu, Jun 25, 2020 at 12:09 PM Yongle Lin 
>> wrote:
>>
>>> Add qp visualization in codecview filter which supports H264 and VP9
>>> codecs. Add options for luma/chroma qp and AC/DC qp as well. There is a old
>>> way to visualize it but it's deprecated since version 58.
>>> example command line to visualize qp:
>>> ./ffmpeg -export_side_data +venc_params -i input.mp4 -vf
>>> codecview=qp=true output.mp4
>>> ---
>>>  doc/filters.texi   |  6 
>>>  libavfilter/vf_codecview.c | 69 +-
>>>  2 files changed, 74 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/doc/filters.texi b/doc/filters.texi
>>> index 84567dec16..f4a57e993f 100644
>>> --- a/doc/filters.texi
>>> +++ b/doc/filters.texi
>>> @@ -7285,6 +7285,12 @@ backward predicted MVs of B-frames
>>>  @item qp
>>>  Display quantization parameters using the chroma planes.
>>>
>>> +@item chroma_qp
>>> +Display chroma quantization parameters (default luma qp) using the
>>> chroma planes. Should use with qp option. (e.g.
>>> codecview=qp=true:chroma_qp=true)
>>> +
>>> +@item dc_qp
>>> +Display DC quantization parameters (default AC qp) using the chroma
>>> planes. Should use with qp option. (e.g. codecview=qp=true:dc_qp=true)
>>> +
>>>  @item mv_type, mvt
>>>  Set motion vectors type to visualize. Includes MVs from all frames
>>> unless specified by @var{frame_type} option.
>>>
>>> diff --git a/libavfilter/vf_codecview.c b/libavfilter/vf_codecview.c
>>> index 331bfba777..f585dfe28e 100644
>>> --- a/libavfilter/vf_codecview.c
>>> +++ b/libavfilter/vf_codecview.c
>>> @@ -34,6 +34,7 @@
>>>  #include "libavutil/opt.h"
>>>  #include "avfilter.h"
>>>  #include "internal.h"
>>> +#include "libavutil/video_enc_params.h"
>>>
>>>  #define MV_P_FOR  (1<<0)
>>>  #define MV_B_FOR  (1<<1)
>>> @@ -51,6 +52,8 @@ typedef struct CodecViewContext {
>>>  unsigned mv_type;
>>>  int hsub, vsub;
>>>  int qp;
>>> +int chroma_qp;
>>> +int dc_qp;
>>>  } CodecViewContext;
>>>
>>>  #define OFFSET(x) offsetof(CodecViewContext, x)
>>> @@ -63,6 +66,8 @@ static const AVOption codecview_options[] = {
>>>  CONST("bf", "forward predicted MVs of B-frames",  MV_B_FOR,
>>> "mv"),
>>>  CONST("bb", "backward predicted MVs of B-frames", MV_B_BACK,
>>> "mv"),
>>>  { "qp", NULL, OFFSET(qp), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, .flags
>>> = FLAGS },
>>> +{ "chroma_qp", NULL, OFFSET(chroma_qp), AV_OPT_TYPE_BOOL, {.i64=0},
>>> 0, 1, .flags = FLAGS },
>>> +{ "dc_qp", NULL, OFFSET(dc_qp), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1,
>>> .flags = FLAGS },
>>>  { "mv_type", "set motion vectors type", OFFSET(mv_type),
>>> AV_OPT_TYPE_FLAGS, {.i64=0}, 0, INT_MAX, FLAGS, "mv_type" },
>>>  { "mvt", "set motion vectors type", OFFSET(mv_type),
>>> AV_OPT_TYPE_FLAGS, {.i64=0}, 0, INT_MAX, FLAGS, "mv_type" },
>>>  CONST("fp", "forward predicted MVs",  MV_TYPE_FOR,  "mv_type"),
>>> @@ -212,6 +217,52 @@ static void draw_arrow(uint8_t *buf, int sx, int
>>> sy, int ex,
>>>  draw_line(buf, sx, sy, ex, ey, w, h, stride, color);
>>>  }
>>>
>>> +static int qp_color_calculate(int qp, enum AVVideoEncParamsType type) {
>>> +return type == AV_VIDEO_ENC_PARAMS_H264 ? qp * 128 / 31 : qp;
>>> +}
>>> +
>>> +static void get_block_color(AVVideoEncParams *par, AVVideoBlockParams
>>> *b, CodecViewContext *s, enum AVColorRange color_range, int *cu, int *cv)
>>> +{
>>> +const int plane_qp_cu_index = s->chroma_qp ? 1 : 0;
>>> +const int plane_qp_cv_index = s->chroma_qp ? 2 : 0;
>>> +const int ac_dc_index = s->dc_qp ? 0 : 1;
>>> +*cu = qp_color_calculate(par->qp +
>>> par->delta_qp[plane_qp_cu_index][ac_dc_index] + b->delta_qp, par->type);
>>> +*cv = qp_color_calculate(par->qp +
>>> par->delta_qp[plane_qp_cv_index][ac_dc_index] + b->delta_qp, par->type);
>>> +if (color_range == AVCOL_RANGE_MPEG) {
>>> +// map jpeg color range(0-255) to mpeg color range(16-235)
>>> +*cu = av_rescale(*cu, 73, 85) + 16;
>>> +*cv = av_rescale(*cv, 73, 85) + 16;
>>> +}
>>> +}
>>> +
>>> +static void color_block(AVFrame *frame, CodecViewContext *s, const int
>>> src_x, const int src_y, const int b_w, const int b_h, const int cu, const
>>> int cv)
>>> +{
>>> +const int w = AV_CEIL_RSHIFT(frame->width,  s->hsub);
>>> +const int h = AV_CEIL_RSHIFT(frame->height, s->vsub);
>>> +const int lzu = frame->linesize[1];
>>> +const int lzv = frame->linesize[2];
>>> +
>>> +const int plane_src_x = src_x >> s->hsub;
>>> +const int plane_src_y = src_y >> s->vsub;
>>> +const int plane_b_w = b_w >> s->hsub;
>>> +const int plane_b_h = b_h >> s->vsub;
>>> +uint8_t *pu = frame->data[1] + plane_src_y * lzu;
>>> +uint8_t *pv = frame->data[2] + plane_src_y * lzv;
>>> +
>>> +for (int y = plane_src_y; y < plane_src_y + plane_b_h; y++) {
>>> +for (int x = plane_src_

Re: [FFmpeg-devel] [PATCH] libavfilter/vf_codecview: add block structure visualization

2020-07-13 Thread Yongle Lin
On Thu, Jul 2, 2020 at 8:09 AM Michael Niedermayer 
wrote:

> On Wed, Jul 01, 2020 at 05:42:48PM +, Yongle Lin wrote:
> > example command line to visualize block decomposition:
> > ./ffmpeg -export_side_data +venc_params -i input.webm -vf
> > codecview=bs=true output.webm
> > ---
>
> >  doc/filters.texi   |  3 +++
> >  libavcodec/vp9.c   |  8 +++-
> >  libavfilter/vf_codecview.c | 41 ++
>
> the vp9 and vf_codecview changes belong in seperate patches, these are 2
> seperate libs
>
> thx
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>

Hey Michael,

I have splitted the patches and sent it to you. Do you have any other
suggestions on this patch? Thanks

Best Regards,
Yongle


>
> In a rich man's house there is no place to spit but his face.
> -- Diogenes of Sinope
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] libavcodec/vp9: export block structure when segmentation isn't enable

2020-07-13 Thread Yongle Lin
On Thu, Jul 9, 2020 at 2:23 PM Yongle Lin  wrote:

>
>
> On Mon, Jul 6, 2020 at 11:31 AM Yongle Lin 
> wrote:
>
>> it makes sense to export block structure like src_x, src_y, width and
>> height when segmentation isn't enable so we could visualize and see the
>> structure of the block.
>> ---
>>  libavcodec/vp9.c | 8 +++-
>>  1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
>> index fd0bab14a2..e700def70e 100644
>> --- a/libavcodec/vp9.c
>> +++ b/libavcodec/vp9.c
>> @@ -1501,10 +1501,8 @@ static int vp9_export_enc_params(VP9Context *s,
>> VP9Frame *frame)
>>  AVVideoEncParams *par;
>>  unsigned int tile, nb_blocks = 0;
>>
>> -if (s->s.h.segmentation.enabled) {
>> -for (tile = 0; tile < s->active_tile_cols; tile++)
>> -nb_blocks += s->td[tile].nb_block_structure;
>> -}
>> +for (tile = 0; tile < s->active_tile_cols; tile++)
>> +nb_blocks += s->td[tile].nb_block_structure;
>>
>>  par = av_video_enc_params_create_side_data(frame->tf.f,
>>  AV_VIDEO_ENC_PARAMS_VP9, nb_blocks);
>> @@ -1536,7 +1534,7 @@ static int vp9_export_enc_params(VP9Context *s,
>> VP9Frame *frame)
>>  b->w = 1 << (3 +
>> td->block_structure[block_tile].block_size_idx_x);
>>  b->h = 1 << (3 +
>> td->block_structure[block_tile].block_size_idx_y);
>>
>> -if (s->s.h.segmentation.feat[seg_id].q_enabled) {
>> +if (s->s.h.segmentation.enabled &&
>> s->s.h.segmentation.feat[seg_id].q_enabled) {
>>  b->delta_qp = s->s.h.segmentation.feat[seg_id].q_val;
>>  if (s->s.h.segmentation.absolute_vals)
>>  b->delta_qp -= par->qp;
>> --
>> 2.27.0.383.g050319c2ae-goog
>>
>>
> Dear FFmpeg Developers,
>
> Currently ffmpeg doesn't export the block data for VP9 if there is no
> segmentation. Because it's only used to export QP value. I think it makes
> more sense to export the block information without segmentation so we could
> visualize the block structure for VP9 video.
>
> Could you please review and merge this patch? Thanks.
>
> Best,
> Yongle
>

Dear FFmpeg Developers,

Currently ffmpeg doesn't export the block data for VP9 if there is no
segmentation. Because it's only used to export QP value. I think it makes
more sense to export the block information without segmentation so we could
visualize the block structure for VP9 video.

Could you please review and merge this patch? Thanks.

Best,
Yongle
___
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 v8 3/3] avdevice/decklink_dec: export timecode with s12m side data

2020-07-13 Thread lance . lmwang
On Mon, Jul 13, 2020 at 10:24:11AM -0400, Devin Heitmueller wrote:
> On Sun, Jul 12, 2020 at 6:16 AM  wrote:
> > I have add fate timecode testing for h264/hevc and haven't submit yet. But 
> > if
> > the frame rate > 30, I got one unexpected result after map SMPTE ST 
> > 12-1:2014
> > side data to HEVC timecode, the frame is 6bit only(2bit for tens of frame),
> > so to framerate > 30, it'll be divided by 2 with same timecode, but HEVC 
> > timecode
> > frame number can use 9bit and expect the frame > 30.
> 
> You should still divide it by 2, even though HEVC supports more bits
> for the frame number.  This is to be consistent with SMPTE 12-1
> timecode, and downstream muxers/outputs aren't going to know whether
> the upstream decoder was H.264 or HEVC.  On the encode side, you can
> multiple by 2 and use the field bit to determine whether to then add
> 1.  See SMPTE ST 12-1:2014 Sec 12.2.

Thanks for the comments, it's helpful, I'll try to fix it on encoder side.

> 
> There's an argument that we should add support for higher framerates
> to support SMPTE 12-3 (which supports fps > 60), but that should be a
> different side data field.  Right now the most important thing is that
> the behavior be consistent across decoders.
> 
> Devin
> 
> -- 
> Devin Heitmueller, Senior Software Engineer
> LTN Global Communications
> o: +1 (301) 363-1001
> w: https://ltnglobal.com  e: devin.heitmuel...@ltnglobal.com
> ___
> 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".

-- 
Thanks,
Limin Wang
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH v3 1/4] libavutil/imgutils: add utility to get plane sizes

2020-07-13 Thread Brian Kim
On Mon, Jul 13, 2020 at 11:22 AM James Almer  wrote:
[...]
> You would need to cast height to size_t for this, i think, but seeing
> av_image_check_size() currently rejects line sizes and plane sizes
> bigger than INT_MAX, maybe we should just keep INT_MAX in the above
> check instead (No need to send a new revision for this, i can amend it
> before pushing with either solution. I just want your opinion).

If we move towards using size_t/ptrdiff_t, it seems to make sense to
use SIZE_MAX so that we do not have to come back and update this
afterwards. It seems like even if we limited the values to INT_MAX
users would think that they should check the range again anyways based
on the type.

However, I am fine with keeping it limited to INT_MAX to keep things
consistent until we can actually use the full range in other places.
___
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] avformat/crypto.c: remove unnecessary code

2020-07-13 Thread 刘歧


在 2020/7/14 上午2:45,“ffmpeg-devel 代表 Tomas 
Härdin” 写入:

lör 2020-07-11 klockan 16:04 +0800 skrev Steven Liu:
> Because the newpos variable is set value before use it.
> The newpos variable declared at the head partition of crypto_seek.
> 
> Signed-off-by: Steven Liu 
> ---
>  libavformat/crypto.c | 11 ---
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/libavformat/crypto.c b/libavformat/crypto.c
> index 31f9ac0ab9..daa29ed501 100644
> --- a/libavformat/crypto.c
> +++ b/libavformat/crypto.c
> @@ -252,21 +252,18 @@ static int64_t crypto_seek(URLContext *h, int64_t 
pos, int whence)
>  case SEEK_CUR:
>  pos = pos + c->position;
>  break;
> -case SEEK_END: {
> -int64_t newpos = ffurl_seek( c->hd, pos, AVSEEK_SIZE );
> +case SEEK_END:
> +newpos = ffurl_seek( c->hd, pos, AVSEEK_SIZE );

Make me wonder why this was declared like this in the first place.
Maybe move the definition of the newpos in the outer scope to where
it's used instead? FFmpeg is C99 now so that should be fine.
I just want make it clean, whatever C89, C99.
Because there had one definition of the newpos at the function start part, and 
assignment value at here before use it at here,
and remove the braces.
Just make it looks clean.
>  if (newpos < 0) {
>  av_log(h, AV_LOG_ERROR,
>  "Crypto: seek_end - can't get file size (pos=%lld)\r\n", 
(long long int)pos);
>  return newpos;
>  }
>  pos = newpos - pos;
> -}
>  break;
> -case AVSEEK_SIZE: {
> -int64_t newpos = ffurl_seek( c->hd, pos, AVSEEK_SIZE );
> +case AVSEEK_SIZE:
> +newpos = ffurl_seek( c->hd, pos, AVSEEK_SIZE );
>  return newpos;

Why not just return ffurl_seek()?
you are right, will submit v2


Thanks

Steven




___
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] libavcodec/jpeg2000dec: Support for PPM marker

2020-07-13 Thread Gautam Ramakrishnan
On Tue, Jul 14, 2020 at 2:57 AM Michael Niedermayer
 wrote:
>
> On Mon, Jul 13, 2020 at 10:50:02PM +0530, gautamr...@gmail.com wrote:
> > From: Gautam Ramakrishnan 
> >
> > This patch adds support for PPM marker for JPEG2000
> > decoder. It allows the samples p1_03.j2k and p1_05.j2k
> > to be decoded.
> > ---
> >  libavcodec/jpeg2000dec.c | 107 +++
> >  1 file changed, 97 insertions(+), 10 deletions(-)
> [...]
>
> > @@ -2189,8 +2266,18 @@ static int 
> > jpeg2000_read_main_headers(Jpeg2000DecoderContext *s)
> >  // Packet length, tile-part header
> >  ret = get_plt(s, len);
> >  break;
> > +case JPEG2000_PPM:
> > +// Packed headers, main header
> > +ret = get_ppm(s, len);
> > +break;
> >  case JPEG2000_PPT:
> >  // Packed headers, tile-part header
> > +if (s->has_ppm) {
> > +av_log(s->avctx, AV_LOG_ERROR,
> > +   "Cannot have both PPT and PPM marker.\n");
> > +return AVERROR_INVALIDDATA;
> > +}
>
> is a similar check needed before get_ppm() ?
As the PPM marker is in the main headers, it has to come before the PPT markers
for sure. However, it strikes me that a check could be added to ensure
that the PPM
markers actually appear only in the main header and not in the tile
part headers.
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> No human being will ever know the Truth, for even if they happen to say it
> by chance, they would not even known they had done so. -- Xenophanes
> ___
> 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".



-- 
-
Gautam |
___
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] avformat/crypto.c: remove unnecessary code

2020-07-13 Thread Steven Liu
Because the newpos variable is set value before use it.
The newpos variable declared at the head partition of crypto_seek.
Make the code clean.

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

diff --git a/libavformat/crypto.c b/libavformat/crypto.c
index 31f9ac0ab9..1d4514e0f2 100644
--- a/libavformat/crypto.c
+++ b/libavformat/crypto.c
@@ -252,21 +252,17 @@ static int64_t crypto_seek(URLContext *h, int64_t pos, 
int whence)
 case SEEK_CUR:
 pos = pos + c->position;
 break;
-case SEEK_END: {
-int64_t newpos = ffurl_seek( c->hd, pos, AVSEEK_SIZE );
+case SEEK_END:
+newpos = ffurl_seek( c->hd, pos, AVSEEK_SIZE );
 if (newpos < 0) {
 av_log(h, AV_LOG_ERROR,
 "Crypto: seek_end - can't get file size (pos=%lld)\r\n", (long 
long int)pos);
 return newpos;
 }
 pos = newpos - pos;
-}
-break;
-case AVSEEK_SIZE: {
-int64_t newpos = ffurl_seek( c->hd, pos, AVSEEK_SIZE );
-return newpos;
-}
 break;
+case AVSEEK_SIZE:
+return ffurl_seek( c->hd, pos, AVSEEK_SIZE );
 default:
 av_log(h, AV_LOG_ERROR,
 "Crypto: no support for seek where 'whence' is %d\r\n", whence);
-- 
2.25.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".