Re: [FFmpeg-devel] [PATCH 07/14] avformat/matroskadec: fix the type of the TrackLanguage

2020-03-25 Thread Andreas Rheinhardt
Steve Lhomme:
> From: Steve Lhomme 
> 
> It's an ASCII string, not a UTF-8 string.
> ---
>  libavformat/matroskadec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index 4d7fdab99f..383869bced 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -554,7 +554,7 @@ static EbmlSyntax matroska_track[] = {
>  { MATROSKA_ID_CODECID,   EBML_STR,   0, 
> offsetof(MatroskaTrack, codec_id) },
>  { MATROSKA_ID_CODECPRIVATE,  EBML_BIN,   0, 
> offsetof(MatroskaTrack, codec_priv) },
>  { MATROSKA_ID_CODECDELAY,EBML_UINT,  0, 
> offsetof(MatroskaTrack, codec_delay) },
> -{ MATROSKA_ID_TRACKLANGUAGE, EBML_UTF8,  0, 
> offsetof(MatroskaTrack, language), { .s = "eng" } },
> +{ MATROSKA_ID_TRACKLANGUAGE, EBML_STR,   0, 
> offsetof(MatroskaTrack, language), { .s = "eng" } },
>  { MATROSKA_ID_TRACKDEFAULTDURATION,  EBML_UINT,  0, 
> offsetof(MatroskaTrack, default_duration) },
>  { MATROSKA_ID_TRACKTIMECODESCALE,EBML_FLOAT, 0, 
> offsetof(MatroskaTrack, time_scale),   { .f = 1.0 } },
>  { MATROSKA_ID_TRACKFLAGDEFAULT,  EBML_UINT,  0, 
> offsetof(MatroskaTrack, flag_default), { .u = 1 } },
> 
Applied, thanks.

- 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 v3] avcodec/jpeg2000dec: error check when processing tlm marker

2020-03-25 Thread gautamramk
From: Gautam Ramakrishnan 

Validate the value of ST field in the TLM marker of JPEG2000.
Throw an error when ST takes value of 0b11.
---
 libavcodec/jpeg2000dec.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
index 019dc81f56..7103cd6ceb 100644
--- a/libavcodec/jpeg2000dec.c
+++ b/libavcodec/jpeg2000dec.c
@@ -795,7 +795,7 @@ static int get_sot(Jpeg2000DecoderContext *s, int n)
  * markers. Parsing the TLM header is needed to increment the input header
  * buffer.
  * This marker is mandatory for DCI. */
-static uint8_t get_tlm(Jpeg2000DecoderContext *s, int n)
+static int get_tlm(Jpeg2000DecoderContext *s, int n)
 {
 uint8_t Stlm, ST, SP, tile_tlm, i;
 bytestream2_get_byte(>g);   /* Ztlm: skipped */
@@ -803,7 +803,11 @@ static uint8_t get_tlm(Jpeg2000DecoderContext *s, int n)
 
 // too complex ? ST = ((Stlm >> 4) & 0x01) + ((Stlm >> 4) & 0x02);
 ST = (Stlm >> 4) & 0x03;
-// TODO: Manage case of ST = 0b11 --> raise error
+if (ST == 0x03) {
+av_log(s->avctx, AV_LOG_ERROR, "TLM marker contains invalid ST 
value.\n");
+return AVERROR_INVALIDDATA;
+}
+
 SP   = (Stlm >> 6) & 0x01;
 tile_tlm = (n - 4) / ((SP + 1) * 2 + ST);
 for (i = 0; i < tile_tlm; i++) {
-- 
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] Add support for playing Audible AAXC (.aaxc) files

2020-03-25 Thread Vesselin Bontchev
From 1a6adbf86fd02775bea467a54c81cc23cc2f6049 Mon Sep 17 00:00:00 2001
From: Vesselin Bontchev 
Date: Sat, 1 Jan 2000 09:00:00 +
Subject: [PATCH] Add support for playing Audible AAXC (.aaxc) files

Note: audible_key and audible_iv values are variable (per file) and are
externally fed.

It is possible to extend https://github.com/mkb79/Audible to derive the
audible_key and audible_key values.

Relevant code:

def decrypt_voucher(deviceSerialNumber, customerId, deviceType, asin, voucher):
buf = (deviceType + deviceSerialNumber + customerId + asin).encode("ascii")
digest = hashlib.sha256(buf).digest()
key = digest[0:16]
iv = digest[16:]

# decrypt "voucher" using AES in CBC mode with no padding
cipher = AES.new(key, AES.MODE_CBC, iv)
plaintext = cipher.decrypt(voucher).rstrip(b"\x00")
return json.loads(plaintext)

The decrypted "voucher" has the required audible_key and audible_iv
values.

Signed-off-by: Vesselin Bontchev 
---
 libavformat/isom.h |  4 
 libavformat/mov.c  | 43 +++
 2 files changed, 47 insertions(+)

diff --git a/libavformat/isom.h b/libavformat/isom.h
index 4943b80ccf..6f7de09155 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -285,6 +285,10 @@ typedef struct MOVContext {
 int activation_bytes_size;
 void *audible_fixed_key;
 int audible_fixed_key_size;
+void *audible_key;
+int audible_key_size;
+void *audible_iv;
+int audible_iv_size;
 struct AVAES *aes_decrypt;
 uint8_t *decryption_key;
 int decryption_key_len;
diff --git a/libavformat/mov.c b/libavformat/mov.c
index f280f360b6..32cb4428a9 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -1084,6 +1084,40 @@ fail:
 return ret;
 }
 
+static int mov_aaxc_crypto(MOVContext *c)
+{
+int ret = 0;
+
+c->aes_decrypt = av_aes_alloc();
+if (!c->aes_decrypt) {
+ret = AVERROR(ENOMEM);
+goto fail;
+}
+
+/* verify audible_key */
+if (c->audible_key_size != 16) {
+av_log(c->fc, AV_LOG_FATAL, "[aaxc] audible_key value needs to be 16 bytes!\n");
+ret = AVERROR(EINVAL);
+goto fail;
+}
+
+/* verify audible_iv */
+if (c->audible_iv_size != 16) {
+av_log(c->fc, AV_LOG_FATAL, "[aaxc] audible_iv value needs to be 16 bytes!\n");
+ret = AVERROR(EINVAL);
+goto fail;
+}
+
+memcpy(c->file_key, c->audible_key, 16);
+memcpy(c->file_iv, c->audible_iv, 16);
+
+c->aax_mode = 1;
+
+fail:
+
+return ret;
+}
+
 // Audible AAX (and AAX+) bytestream decryption
 static int aax_filter(uint8_t *input, int size, MOVContext *c)
 {
@@ -1132,6 +1166,11 @@ static int mov_read_ftyp(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 av_dict_set(>fc->metadata, "compatible_brands",
 comp_brands_str, AV_DICT_DONT_STRDUP_VAL);
 
+// Logic for handling Audible's .aaxc files
+if (!strcmp(type, "aaxc")) {
+mov_aaxc_crypto(c);
+}
+
 return 0;
 }
 
@@ -8068,6 +8107,10 @@ static const AVOption mov_options[] = {
 AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, .flags = FLAGS },
 { "activation_bytes", "Secret bytes for Audible AAX files", OFFSET(activation_bytes),
 AV_OPT_TYPE_BINARY, .flags = AV_OPT_FLAG_DECODING_PARAM },
+{ "audible_key", "AES-128 Key for Audible AAXC files", OFFSET(audible_key),
+AV_OPT_TYPE_BINARY, .flags = AV_OPT_FLAG_DECODING_PARAM },
+{ "audible_iv", "AES-128 IV for Audible AAXC files", OFFSET(audible_iv),
+AV_OPT_TYPE_BINARY, .flags = AV_OPT_FLAG_DECODING_PARAM },
 { "audible_fixed_key", // extracted from libAAX_SDK.so and AAXSDKWin.dll files!
 "Fixed key used for handling Audible AAX files", OFFSET(audible_fixed_key),
 AV_OPT_TYPE_BINARY, {.str="77214d4b196a87cd520045fd20a51d67"},
-- 
2.26.0

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

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

[FFmpeg-devel] [PATCH 2/3] avformat/wvdec: Export version as extradata

2020-03-25 Thread Andreas Rheinhardt
It might be used by the Matroska muxer. This is also the reason why the
FATE-tests for muxing WavPack into Matroska needed to be updated: They
now write the correct version 4.07 and not 4.03 as before.

Signed-off-by: Andreas Rheinhardt 
---
 libavformat/wvdec.c| 3 +++
 tests/fate/wavpack.mak | 4 ++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/libavformat/wvdec.c b/libavformat/wvdec.c
index e56a6932ad..b9fc6a59f9 100644
--- a/libavformat/wvdec.c
+++ b/libavformat/wvdec.c
@@ -250,6 +250,9 @@ static int wv_read_header(AVFormatContext *s)
 st = avformat_new_stream(s, NULL);
 if (!st)
 return AVERROR(ENOMEM);
+if ((ret = ff_alloc_extradata(st->codecpar, 2)) < 0)
+return ret;
+AV_WL16(st->codecpar->extradata, wc->header.version);
 st->codecpar->codec_type= AVMEDIA_TYPE_AUDIO;
 st->codecpar->codec_id  = AV_CODEC_ID_WAVPACK;
 st->codecpar->channels  = wc->chan;
diff --git a/tests/fate/wavpack.mak b/tests/fate/wavpack.mak
index a89cc1293d..bc7a5cc92f 100644
--- a/tests/fate/wavpack.mak
+++ b/tests/fate/wavpack.mak
@@ -91,12 +91,12 @@ fate-wavpack-matroskamode: CMD = md5 -i 
$(TARGET_SAMPLES)/wavpack/special/matros
 FATE_WAVPACK-$(call DEMMUX, WV, MATROSKA) += fate-wavpack-matroska_mux-mono
 fate-wavpack-matroska_mux-mono: CMD = md5pipe -i 
$(TARGET_SAMPLES)/wavpack/num_channels/mono_16bit_int.wv -c copy -fflags 
+bitexact -f matroska
 fate-wavpack-matroska_mux-mono: CMP = oneline
-fate-wavpack-matroska_mux-mono: REF = 0a1a7c6d3b413925f8261c92d1d53363
+fate-wavpack-matroska_mux-mono: REF = a9da0444604848080b08e32de19f42d9
 
 FATE_WAVPACK-$(call DEMMUX, WV, MATROSKA) += fate-wavpack-matroska_mux-61
 fate-wavpack-matroska_mux-61: CMD = md5pipe -i 
$(TARGET_SAMPLES)/wavpack/num_channels/eva_2.22_6.1_16bit-partial.wv -c copy 
-fflags +bitexact -f matroska
 fate-wavpack-matroska_mux-61: CMP = oneline
-fate-wavpack-matroska_mux-61: REF = 1f012f7c237f46f2f63089ba84c589c5
+fate-wavpack-matroska_mux-61: REF = 60812e377937456863a3c899ee068060
 
 FATE_SAMPLES_AVCONV += $(FATE_WAVPACK-yes)
 fate-wavpack: $(FATE_WAVPACK-yes)
-- 
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 3/3] avformat/matroskaenc: Update the default version of WavPack

2020-03-25 Thread Andreas Rheinhardt
The Matroska muxer currently assumed WavPack version 4.03 in case it was
not explicitly signalled via extradata; but following a recommendation
by David Bryant, the WavPack creator, this is changed to 4.10.

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

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index f9e69c6c89..725282770e 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -612,7 +612,7 @@ static int put_wv_codecpriv(AVIOContext *pb, 
AVCodecParameters *par)
 if (par->extradata && par->extradata_size == 2)
 avio_write(pb, par->extradata, 2);
 else
-avio_wl16(pb, 0x403); // fallback to the version mentioned in matroska 
specs
+avio_wl16(pb, 0x410); // fallback to the most recent version
 return 0;
 }
 
-- 
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 1/3] avformat/matroskadec: Add a workaround for missing WavPack extradata

2020-03-25 Thread Andreas Rheinhardt
mkvmerge versions 6.2 to 40.0 had a bug that made it not propagate the
WavPack extradata (containing the WavPack version) during remuxing from
a Matroska file; currently our demuxer would treat every WavPack block
encountered as invalid data (unless the WavPack stream is to be
discarded (i.e. the streams discard is >= AVDISCARD_ALL)) and try to
resync to the next level 1 element.

Luckily, the WavPack version is currently not really important; so we
fix this problem by assuming a version. David Bryant, the creator of
WavPack, recommended using version 0x410 (the most recent version) for
this. And this is what this commit does.

A FATE-test for this has been added.

Signed-off-by: Andreas Rheinhardt 
---
James has already uploaded the sample. Thanks for this.

 libavformat/matroskadec.c| 11 ++-
 tests/fate/matroska.mak  |  5 +
 tests/ref/fate/matroska-wavpack-missing-codecprivate |  9 +
 3 files changed, 24 insertions(+), 1 deletion(-)
 create mode 100644 tests/ref/fate/matroska-wavpack-missing-codecprivate

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 4d7fdab99f..5b55606b98 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -2613,6 +2613,14 @@ static int matroska_parse_tracks(AVFormatContext *s)
 ret = matroska_parse_flac(s, track, _offset);
 if (ret < 0)
 return ret;
+} else if (codec_id == AV_CODEC_ID_WAVPACK && track->codec_priv.size < 
2) {
+av_log(matroska->ctx, AV_LOG_INFO, "Assuming WavPack version 4.10 "
+   "in absence of valid CodecPrivate.\n");
+extradata_size = 2;
+extradata = av_mallocz(2 + AV_INPUT_BUFFER_PADDING_SIZE);
+if (!extradata)
+return AVERROR(ENOMEM);
+AV_WL16(extradata, 0x410);
 } else if (codec_id == AV_CODEC_ID_PRORES && track->codec_priv.size == 
4) {
 fourcc = AV_RL32(track->codec_priv.data);
 } else if (codec_id == AV_CODEC_ID_VP9 && track->codec_priv.size) {
@@ -3165,9 +3173,10 @@ static int matroska_parse_wavpack(MatroskaTrack *track, 
uint8_t *src,
 uint16_t ver;
 int ret, offset = 0;
 
-if (srclen < 12 || track->stream->codecpar->extradata_size < 2)
+if (srclen < 12)
 return AVERROR_INVALIDDATA;
 
+av_assert1(track->stream->codecpar->extradata_size >= 2);
 ver = AV_RL16(track->stream->codecpar->extradata);
 
 samples = AV_RL32(src);
diff --git a/tests/fate/matroska.mak b/tests/fate/matroska.mak
index b9ed7322fd..93b5bff89a 100644
--- a/tests/fate/matroska.mak
+++ b/tests/fate/matroska.mak
@@ -17,6 +17,11 @@ fate-matroska-remux: REF = 49a60ef91cf7302ab7276f9373f8a429
 FATE_MATROSKA-$(call ALLYES, MATROSKA_DEMUXER VORBIS_PARSER) += 
fate-matroska-xiph-lacing
 fate-matroska-xiph-lacing: CMD = framecrc -i 
$(TARGET_SAMPLES)/mkv/xiph_lacing.mka -c:a copy
 
+# This tests that the Matroska demuxer correctly demuxes WavPack
+# without CodecPrivate; it also tests zlib compressed WavPack.
+FATE_MATROSKA-$(call ALLYES, MATROSKA_DEMUXER ZLIB) += 
fate-matroska-wavpack-missing-codecprivate
+fate-matroska-wavpack-missing-codecprivate: CMD = framecrc -i 
$(TARGET_SAMPLES)/mkv/wavpack_missing_codecprivate.mka -c copy
+
 # This tests that the matroska demuxer supports decompressing
 # zlib compressed tracks (both the CodecPrivate as well as the actual frames).
 FATE_MATROSKA-$(call ALLYES, MATROSKA_DEMUXER ZLIB) += 
fate-matroska-zlib-decompression
diff --git a/tests/ref/fate/matroska-wavpack-missing-codecprivate 
b/tests/ref/fate/matroska-wavpack-missing-codecprivate
new file mode 100644
index 00..4645a86ff6
--- /dev/null
+++ b/tests/ref/fate/matroska-wavpack-missing-codecprivate
@@ -0,0 +1,9 @@
+#extradata 0:2, 0x00240014
+#tb 0: 11337/5
+#media_type 0: audio
+#codec_id 0: wavpack
+#sample_rate 0: 44100
+#channel_layout 0: 3
+#channel_layout_name 0: stereo
+0,  0,  0,22051,14778, 0x02819286
+0,  22051,  22051,22052,14756, 0x21976243
-- 
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 05/17] avformat/webm_chunk: Don't use child AVFormatContext for logging

2020-03-25 Thread Andreas Rheinhardt
Andreas Rheinhardt:
> Andreas Rheinhardt:
>> Signed-off-by: Andreas Rheinhardt 
>> ---
>>  libavformat/webm_chunk.c | 7 +++
>>  1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/libavformat/webm_chunk.c b/libavformat/webm_chunk.c
>> index bc3d346a00..0d4e3598ed 100644
>> --- a/libavformat/webm_chunk.c
>> +++ b/libavformat/webm_chunk.c
>> @@ -87,25 +87,24 @@ static int chunk_mux_init(AVFormatContext *s)
>>  static int get_chunk_filename(AVFormatContext *s, int is_header, char 
>> filename[MAX_FILENAME_SIZE])
>>  {
>>  WebMChunkContext *wc = s->priv_data;
>> -AVFormatContext *oc = wc->avf;
>>  if (!filename) {
>>  return AVERROR(EINVAL);
>>  }
>>  if (is_header) {
>>  int len;
>>  if (!wc->header_filename) {
>> -av_log(oc, AV_LOG_ERROR, "No header filename provided\n");
>> +av_log(s, AV_LOG_ERROR, "No header filename provided\n");
>>  return AVERROR(EINVAL);
>>  }
>>  len = av_strlcpy(filename, wc->header_filename, MAX_FILENAME_SIZE);
>>  if (len >= MAX_FILENAME_SIZE) {
>> -av_log(oc, AV_LOG_ERROR, "Header filename too long\n");
>> +av_log(s, AV_LOG_ERROR, "Header filename too long\n");
>>  return AVERROR(EINVAL);
>>  }
>>  } else {
>>  if (av_get_frame_filename(filename, MAX_FILENAME_SIZE,
>>s->url, wc->chunk_index - 1) < 0) {
>> -av_log(oc, AV_LOG_ERROR, "Invalid chunk filename template 
>> '%s'\n", s->url);
>> +av_log(s, AV_LOG_ERROR, "Invalid chunk filename template 
>> '%s'\n", s->url);
>>  return AVERROR(EINVAL);
>>  }
>>  }
>>
> Ping. If no one objects, I will merge the webm_chunk patches tomorrow.
> 
> - Andreas
> 
Pushed.

- 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 3/3] avformat/matroskadec: Improve forward compability

2020-03-25 Thread Andreas Rheinhardt
Matroska is built around the principle that a reader does not need to
understand everything in a file in order to be able to make use of it;
it just needs to ignore the data it doesn't know about.

Our demuxer typically follows this principle, but there is one important
instance where it does not: A Block belonging to a TrackEntry with no
associated stream is treated as invalid data (i.e. the demuxer will try
to resync to the next level 1 element because it takes this as a sign
that it has lost sync). Given that we do not create streams if we don't
know or don't support the type of the TrackEntry, this impairs this
demuxer's forward compability.

Furthermore, ignoring Blocks belonging to a TrackEntry without
corresponding stream can (in future commits) also be used to ignore
TrackEntries with obviously bogus entries without affecting the other
TrackEntries (by not creating a stream for said TrackEntry).

Finally, given that matroska_find_track_by_num() already emits its own
error message in case there is no TrackEntry with a given TrackNumber,
the error message (with level AV_LOG_INFO) for this can be removed.

Signed-off-by: Andreas Rheinhardt 
---
 libavformat/matroskadec.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 7aea13dda0..cd85d3e281 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -3522,13 +3522,16 @@ static int matroska_parse_block(MatroskaDemuxContext 
*matroska, AVBufferRef *buf
 size -= n;
 
 track = matroska_find_track_by_num(matroska, num);
-if (!track || !track->stream) {
-av_log(matroska->ctx, AV_LOG_INFO,
-   "Invalid stream %"PRIu64"\n", num);
+if (!track || size < 3)
 return AVERROR_INVALIDDATA;
-} else if (size < 3)
-return AVERROR_INVALIDDATA;
-st = track->stream;
+
+if (!(st = track->stream)) {
+av_log(matroska->ctx, AV_LOG_VERBOSE,
+   "No stream associated to TrackNumber %"PRIu64". "
+   "Ignoring Block with this TrackNumber.\n", num);
+return 0;
+}
+
 if (st->discard >= AVDISCARD_ALL)
 return res;
 av_assert1(block_duration != AV_NOPTS_VALUE);
-- 
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/3] avformat/matroskadec: Don't discard valid packets

2020-03-25 Thread Andreas Rheinhardt
A Block (meaning both a Block in a BlockGroup as well as a SimpleBlock)
must have at least three bytes after the field containing the encoded
TrackNumber. So a if there are <= 3 bytes, the Matroska demuxer would
skip this block, believing it to be an empty, but valid Block.

This might discard valid Blocks, even nonempty ones (namely if the track
uses header stripping). And certain definitely spec-incompliant Blocks
don't raise errors: Those with two or less bytes left after the encoded
TrackNumber and those with three bytes left, but with flags indicating
that the Block uses lacing (in which case there has to be further data
describing the lacing).

Furthermore, zero-sized packets were still possible because only the
size of the last entry of a lace was checked.

This commit fixes this. All spec-compliant Blocks are now returned to
the caller, even those with a size of zero. This is in accordance with the
documentation of av_read_frame(): "This function returns what is stored
in the file, and does not validate that what is there are valid frames
for the decoder".

Signed-off-by: Andreas Rheinhardt 
---
mkclean can produce blocks where the payload has a size of zero before
readding the removed header. E.g. it does this if a stream only has one
frame in total (although in such a case the overhead of header removal
compression is greater than the number of bytes saved in the Blocks);
this can in particular happen with forced subtitle tracks with only one
frame (which is how I noticed this).

I am unsure what to do with size-zero packets; I don't know any
Matroska Codec Mapping where this would be allowed. But there is nothing
explicitly stating that they are generally illegal, so this commit
returns them. The only thing I am certain is that stripping these
packets away needs to happen after the content compressions have been
reversed.

 libavformat/matroskadec.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 6425973954..7aea13dda0 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -2991,7 +2991,9 @@ static int matroska_parse_laces(MatroskaDemuxContext 
*matroska, uint8_t **buf,
 return 0;
 }
 
-av_assert0(size > 0);
+if (size <= 0)
+return AVERROR_INVALIDDATA;
+
 *laces= *data + 1;
 data += 1;
 size -= 1;
@@ -3017,7 +3019,7 @@ static int matroska_parse_laces(MatroskaDemuxContext 
*matroska, uint8_t **buf,
 break;
 }
 }
-if (size <= total) {
+if (size < total) {
 return AVERROR_INVALIDDATA;
 }
 
@@ -3064,7 +3066,7 @@ static int matroska_parse_laces(MatroskaDemuxContext 
*matroska, uint8_t **buf,
 }
 data += offset;
 size -= offset;
-if (size <= total) {
+if (size < total) {
 return AVERROR_INVALIDDATA;
 }
 lace_size[*laces - 1] = size - total;
@@ -3524,8 +3526,8 @@ static int matroska_parse_block(MatroskaDemuxContext 
*matroska, AVBufferRef *buf
 av_log(matroska->ctx, AV_LOG_INFO,
"Invalid stream %"PRIu64"\n", num);
 return AVERROR_INVALIDDATA;
-} else if (size <= 3)
-return 0;
+} else if (size < 3)
+return AVERROR_INVALIDDATA;
 st = track->stream;
 if (st->discard >= AVDISCARD_ALL)
 return res;
-- 
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 1/3] avformat/matroskadec: Don't discard the upper 32bits of TrackNumber

2020-03-25 Thread Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt 
---
This has already been sent in [1]; I'm resending it because it fits
here.

[1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2019-December/254841.html

 libavformat/matroskadec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 4d7fdab99f..6425973954 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -1545,7 +1545,7 @@ static int matroska_probe(const AVProbeData *p)
 }
 
 static MatroskaTrack *matroska_find_track_by_num(MatroskaDemuxContext 
*matroska,
- int num)
+ uint64_t num)
 {
 MatroskaTrack *tracks = matroska->tracks.elem;
 int i;
@@ -1554,7 +1554,7 @@ static MatroskaTrack 
*matroska_find_track_by_num(MatroskaDemuxContext *matroska,
 if (tracks[i].num == num)
 return [i];
 
-av_log(matroska->ctx, AV_LOG_ERROR, "Invalid track number %d\n", num);
+av_log(matroska->ctx, AV_LOG_ERROR, "Invalid track number %"PRIu64"\n", 
num);
 return NULL;
 }
 
-- 
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 v1 1/3] avfilter/af_acrossover: Check sscanf() return value

2020-03-25 Thread lance . lmwang
From: Limin Wang 

Signed-off-by: Limin Wang 
---
 libavfilter/af_acrossover.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libavfilter/af_acrossover.c b/libavfilter/af_acrossover.c
index 20d1d2bda9..c9404b95cd 100644
--- a/libavfilter/af_acrossover.c
+++ b/libavfilter/af_acrossover.c
@@ -98,7 +98,8 @@ static av_cold int init(AVFilterContext *ctx)
 
 p = NULL;
 
-av_sscanf(arg, "%f", );
+if (av_sscanf(arg, "%f", ) != 1)
+return AVERROR(EINVAL);
 if (freq <= 0) {
 av_log(ctx, AV_LOG_ERROR, "Frequency %f must be positive 
number.\n", freq);
 return AVERROR(EINVAL);
-- 
2.21.0

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

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

[FFmpeg-devel] [PATCH]configure: Get the correct ident for clang-cl.exe

2020-03-25 Thread Carl Eugen Hoyos
Hi!

Attached patch avoids that ffmpeg claims its compiler was "No input
file" when using clang-cl.

Please comment, Carl Eugen
From 659397c2ef2881cc5a5c99597aab5946a8f28c43 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos 
Date: Thu, 26 Mar 2020 00:00:10 +0100
Subject: [PATCH] configure: Get the correct ident for clang-cl.exe

Instead of "No input file specified"
---
 configure | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 18f2841765..68a81f81bb 100755
--- a/configure
+++ b/configure
@@ -4663,7 +4663,11 @@ probe_cc(){
 _ld_path='-libpath:'
 elif $_cc -nologo- 2>&1 | grep -q Microsoft || { $_cc -v 2>&1 | grep -q clang && $_cc -? > /dev/null 2>&1; }; then
 _type=msvc
-_ident=$($_cc 2>&1 | head -n1 | tr -d '\r')
+if $_cc -nologo- 2>&1 | grep -q Microsoft; then
+_ident=$($_cc 2>&1 | head -n1 | tr -d '\r')
+else
+_ident=$($_cc --version 2>/dev/null | head -n1)
+fi
 _DEPCMD='$(DEP$(1)) $(DEP$(1)FLAGS) $($(1)DEP_FLAGS) $< 2>&1 | awk '\''/including/ { sub(/^.*file: */, ""); gsub(/\\/, "/"); if (!match($$0, / /)) print "$@:", $$0 }'\'' > $(@:.o=.d)'
 _DEPFLAGS='$(CPPFLAGS) $(CFLAGS) -showIncludes -Zs'
 _cflags_speed="-O2"
-- 
2.24.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]configure: Remove all C standard versions from the MSVC command line

2020-03-25 Thread Carl Eugen Hoyos
Hi!

Attached patch removes an ugly warning shown for every file when using
clang-cl.exe.

Please comment, Carl Eugen
From 61a07641e2babbb3bc0ad6375d1cb9934f8b434a Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos 
Date: Wed, 25 Mar 2020 23:59:11 +0100
Subject: [PATCH] configure: Remove all C standard versions from the MSVC
 command line.

Silences a warning for every file when compiling with clang-cl.exe
---
 configure | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 18f2841765..68a81f81bb 100755
--- a/configure
+++ b/configure
@@ -4420,7 +4420,7 @@ msvc_common_flags(){
 # generic catch all at the bottom will print the original flag.
 -Wall);;
 -Wextra)  ;;
--std=c99) ;;
+-std=c*)  ;;
 # Common flags
 -fomit-frame-pointer) ;;
 -g)   echo -Z7 ;;
___
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 v1 3/3] avfilter/af_adelay: Check sscanf() return value

2020-03-25 Thread lance . lmwang
From: Limin Wang 

Signed-off-by: Limin Wang 
---
 libavfilter/af_adelay.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libavfilter/af_adelay.c b/libavfilter/af_adelay.c
index c9647771f2..7f498ba190 100644
--- a/libavfilter/af_adelay.c
+++ b/libavfilter/af_adelay.c
@@ -155,7 +155,8 @@ static int config_input(AVFilterLink *inlink)
 ret = av_sscanf(arg, "%d%c", >delay, );
 if (ret != 2 || type != 'S') {
 div = type == 's' ? 1.0 : 1000.0;
-av_sscanf(arg, "%f", );
+if (av_sscanf(arg, "%f", ) != 1)
+return AVERROR(EINVAL);
 d->delay = delay * inlink->sample_rate / div;
 }
 
-- 
2.21.0

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

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

[FFmpeg-devel] [PATCH v1 2/3] avfilter/vf_mix: Check sscanf() return value

2020-03-25 Thread lance . lmwang
From: Limin Wang 

Signed-off-by: Limin Wang 
---
 libavfilter/vf_mix.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libavfilter/vf_mix.c b/libavfilter/vf_mix.c
index 9e1ae79e00..e47f7e2301 100644
--- a/libavfilter/vf_mix.c
+++ b/libavfilter/vf_mix.c
@@ -108,7 +108,8 @@ static av_cold int init(AVFilterContext *ctx)
 break;
 
 p = NULL;
-av_sscanf(arg, "%f", >weights[i]);
+if (av_sscanf(arg, "%f", >weights[i]) != 1)
+continue;
 s->wfactor += s->weights[i];
 last = i;
 }
-- 
2.21.0

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

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

Re: [FFmpeg-devel] [PATCH] avformat/webmdashenc: Don't use custom option for bitexactness

2020-03-25 Thread Andreas Rheinhardt
Andreas Rheinhardt:
> Andreas Rheinhardt:
>> The WebM DASH Manifest muxer can write manifests for live streams and
>> these contain an entry that depends on the time the manifest is written;
>> an AVOption to make the output reproducible has been added for tests.
>> But this is unnecessary, as there already is a method for reproducible
>> output: The AVFMT_FLAG_BITEXACT-flag of the AVFormatContext. Therefore
>> this commit removes the custom option.
>>
>> Given that the description of said option contained "private option -
>> users should never set this" and that it was not documented in
>> muxers.texi, no deprecation period for this option seemed necessary.
>>
>> The commands of the FATE-tests for this muxer have been changed to no
>> longer use this option.
>>
>> Signed-off-by: Andreas Rheinhardt 
>> ---
>>  libavformat/webmdashenc.c | 4 +---
>>  tests/fate/vpx.mak| 4 ++--
>>  2 files changed, 3 insertions(+), 5 deletions(-)
>>
[...]
>>
> 
> If no one objects, I'll push this tomorrow.
> 
> - Andreas
> 
Applied.

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

Re: [FFmpeg-devel] [PATCH 02/14] avformat/matroska: clean the structure formatting

2020-03-25 Thread Andreas Rheinhardt
Steve Lhomme:
> From: Steve Lhomme 
> 
> Always use a comma at the end, order elements by value.
> ---
>  libavformat/matroska.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/matroska.h b/libavformat/matroska.h
> index 9e33e51c94..e177cd027f 100644
> --- a/libavformat/matroska.h
> +++ b/libavformat/matroska.h
> @@ -286,13 +286,13 @@ typedef enum {
>  typedef enum {
>  MATROSKA_VIDEO_INTERLACE_FLAG_UNDETERMINED = 0,
>  MATROSKA_VIDEO_INTERLACE_FLAG_INTERLACED   = 1,
> -MATROSKA_VIDEO_INTERLACE_FLAG_PROGRESSIVE  = 2
> +MATROSKA_VIDEO_INTERLACE_FLAG_PROGRESSIVE  = 2,
>  } MatroskaVideoInterlaceFlag;
>  
>  typedef enum {
>  MATROSKA_VIDEO_FIELDORDER_PROGRESSIVE  = 0,
> -MATROSKA_VIDEO_FIELDORDER_UNDETERMINED = 2,
>  MATROSKA_VIDEO_FIELDORDER_TT   = 1,
> +MATROSKA_VIDEO_FIELDORDER_UNDETERMINED = 2,
>  MATROSKA_VIDEO_FIELDORDER_BB   = 6,
>  MATROSKA_VIDEO_FIELDORDER_TB   = 9,
>  MATROSKA_VIDEO_FIELDORDER_BT   = 14,
> 
Would it actually be allowed to add new values to the range of
FlagInterlaced (to which the MatroskaVideoInterlaceFlag enum
corresponds)? If no, then we should not add a comma, as this signals
extensibility.
The reordering looks good either way.

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

Re: [FFmpeg-devel] [PATCH] Check return value from avio_read() to verify data actually read

2020-03-25 Thread John Rummell
>
> These would cause mov_read_adrm() to fail but not neccessarily return an
> error code if any of these reads less.
> Is that intended ?


Not at all. Updated to always return AVERROR_INVALIDDATA.

On Tue, Mar 24, 2020 at 6:00 PM Michael Niedermayer 
wrote:

> On Mon, Mar 23, 2020 at 05:52:01PM -0700, John Rummell wrote:
> > Chromium fuzzers have caught places where uninitialized data was used due
> > to calls to avio_read() not verifying that the number of bytes expected
> was
> > actually read. So updating the code to check the result from avio_read().
>
> >  mov.c|   26 ++
> >  oggdec.c |3 ++-
> >  wavdec.c |   12 
> >  3 files changed, 28 insertions(+), 13 deletions(-)
> > 66938bc5adfc7d151b376f6d396c4a0dc7f97a4c
> 0001-Check-return-value-from-avio_read-to-verify-data-act.patch
> > From 7f80d50711486a4b923bd8d1e26abc9649d570e3 Mon Sep 17 00:00:00 2001
> > From: John Rummell 
> > Date: Mon, 23 Mar 2020 15:48:33 -0700
> > Subject: [PATCH] Check return value from avio_read() to verify data
> actually
> >  read
> >
> > If the buffer doesn't contain enough bytes when reading a stream,
> > fail rather than continuing on with unitialized data. One attempt
> > caught by Chromium fuzzers (crbug.com/1054229), rest done by looking
> > for calls to avio_read() that don't check the result in Chromium
> > code search.
> > ---
> >  libavformat/mov.c| 26 ++
> >  libavformat/oggdec.c |  3 ++-
> >  libavformat/wavdec.c | 12 
> >  3 files changed, 28 insertions(+), 13 deletions(-)
> >
> > diff --git a/libavformat/mov.c b/libavformat/mov.c
> > index f280f360b6..a5b4d04e37 100644
> > --- a/libavformat/mov.c
> > +++ b/libavformat/mov.c
> > @@ -1012,10 +1012,16 @@ static int mov_read_adrm(MOVContext *c,
> AVIOContext *pb, MOVAtom atom)
> >  }
> >
> >  /* drm blob processing */
> > -avio_read(pb, output, 8); // go to offset 8, absolute position 0x251
> > -avio_read(pb, input, DRM_BLOB_SIZE);
> > -avio_read(pb, output, 4); // go to offset 4, absolute position 0x28d
> > -avio_read(pb, file_checksum, 20);
> > +/* go to offset 8, absolute position 0x251 */
> > +if ((ret = avio_read(pb, output, 8)) != 8)
> > +goto fail;
> > +if ((ret = avio_read(pb, input, DRM_BLOB_SIZE)) != DRM_BLOB_SIZE)
> > +goto fail;
> > +/* go to offset 4, absolute position 0x28d */
> > +if ((ret = avio_read(pb, output, 4)) != 4)
> > +goto fail;
> > +if ((ret = avio_read(pb, file_checksum, 20)) != 20)
> > +goto fail;
>
> These would cause mov_read_adrm() to fail but not neccessarily return an
> error code if any of these reads less.
> Is that intended ?
>
> thx
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Complexity theory is the science of finding the exact solution to an
> approximation. Benchmarking OTOH is finding an approximation of the exact
> ___
> 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".


0002-Check-return-value-from-avio_read-to-verify-data-act.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 01/14] avformat/matroska: add missing Buttons track type

2020-03-25 Thread Andreas Rheinhardt
Steve Lhomme:
> From: Steve Lhomme 
> 
> ---
>  libavformat/matroska.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libavformat/matroska.h b/libavformat/matroska.h
> index 86968a8de1..9e33e51c94 100644
> --- a/libavformat/matroska.h
> +++ b/libavformat/matroska.h
> @@ -271,6 +271,7 @@ typedef enum {
>MATROSKA_TRACK_TYPE_COMPLEX  = 0x3,
>MATROSKA_TRACK_TYPE_LOGO = 0x10,
>MATROSKA_TRACK_TYPE_SUBTITLE = 0x11,
> +  MATROSKA_TRACK_TYPE_BUTTONS  = 0x12,
>MATROSKA_TRACK_TYPE_CONTROL  = 0x20,
>MATROSKA_TRACK_TYPE_METADATA = 0x21,
>  } MatroskaTrackType;
> 
Applied, thanks.

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

Re: [FFmpeg-devel] [PATCH v2] avformat/matroskaenc: Write level 1 elements in one go

2020-03-25 Thread Carl Eugen Hoyos
Am Mi., 25. März 2020 um 18:08 Uhr schrieb Andreas Rheinhardt
:
>
> Carl Eugen Hoyos:
> > Am Mi., 25. März 2020 um 05:45 Uhr schrieb Andreas Rheinhardt
> > :
> >>
> >> Up until now, writing level 1 elements proceeded as follows: First, the
> >> element id was written to the ordinary output AVIOContext and a dynamic
> >> buffer was opened for the content of the level 1 element in
> >> start_ebml_master_crc32(). Then this buffer was actually used and after it
> >> was closed (in end_ebml_master_crc32()), the size field corresponding to
> >> the buffer's size was written, after which the actual data was written.
> >>
> >> This commit changes this: Nothing is written to the main AVIOContext any
> >> more in start_ebml_master_crc32(). end_ebml_master_crc32() now writes
> >> both the id, the length field as well as the data. This implies that
> >> one can start a level 1 element in memory without outputting anything.
> >> This is done to enable to test whether enough space has been reserved
> >> for the Cues (if space has been reserved for them) before writing them.
> >> A large duration between outputting the header and outputting the rest
> >> could also break certain streaming usecases like the one from #8578
> >> (which this commit fixes).
> >>
> >> Signed-off-by: Andreas Rheinhardt 
> >> ---
> >> The commit message has been updated in light of ticket #8578; nothing
> >> else has changed.
> >
> > Please mention the ticket in the commit message.
> >
> It is already contained in the commit message.

Found it.

Sorry, 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] [PATCH v2] avcodec/jpeg2000dec: error check when processing tlm marker

2020-03-25 Thread gautamramk
From: Gautam Ramakrishnan 

Validate the value of ST field in the TLM marker of JPEG2000.
Throw an error when ST takes value of 0x11.
---
 libavcodec/jpeg2000dec.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
index 019dc81f56..a233bcafc7 100644
--- a/libavcodec/jpeg2000dec.c
+++ b/libavcodec/jpeg2000dec.c
@@ -795,7 +795,7 @@ static int get_sot(Jpeg2000DecoderContext *s, int n)
  * markers. Parsing the TLM header is needed to increment the input header
  * buffer.
  * This marker is mandatory for DCI. */
-static uint8_t get_tlm(Jpeg2000DecoderContext *s, int n)
+static int get_tlm(Jpeg2000DecoderContext *s, int n)
 {
 uint8_t Stlm, ST, SP, tile_tlm, i;
 bytestream2_get_byte(>g);   /* Ztlm: skipped */
@@ -803,7 +803,11 @@ static uint8_t get_tlm(Jpeg2000DecoderContext *s, int n)
 
 // too complex ? ST = ((Stlm >> 4) & 0x01) + ((Stlm >> 4) & 0x02);
 ST = (Stlm >> 4) & 0x03;
-// TODO: Manage case of ST = 0b11 --> raise error
+if (ST == 0x03) {
+av_log(s, AV_LOG_ERROR, "TLM marker contains invalid ST value.\n");
+return AVERROR_INVALIDDATA;
+}
+
 SP   = (Stlm >> 6) & 0x01;
 tile_tlm = (n - 4) / ((SP + 1) * 2 + ST);
 for (i = 0; i < tile_tlm; i++) {
-- 
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] avformat/movenc: Reduce size of the allocated MOVIentry array

2020-03-25 Thread James Almer
Increase it in a linear way instead.
Helps reduce memory usage, especially on long, non fragmented output.

Signed-off-by: James Almer 
---
An alternative is using av_fast_realloc(), in a similar fashion as
ff_add_index_entry(), which will keep the memory usage more closely tied to the
amount of entries needed, but the amount of reallocations will be much higher,
so i don't know if the tradeof is beneficial.

 libavformat/movenc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 85df5d1374..ce82acf914 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -5559,7 +5559,7 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
 }
 
 if (trk->entry >= trk->cluster_capacity) {
-unsigned new_capacity = 2 * (trk->entry + MOV_INDEX_CLUSTER_SIZE);
+unsigned new_capacity = trk->entry + MOV_INDEX_CLUSTER_SIZE;
 if (av_reallocp_array(>cluster, new_capacity,
   sizeof(*trk->cluster))) {
 ret = AVERROR(ENOMEM);
-- 
2.25.2

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

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

Re: [FFmpeg-devel] [PATCH] avcodec/jpeg2000dec: error check when processing tlm marker

2020-03-25 Thread Michael Niedermayer
On Wed, Mar 25, 2020 at 08:15:11AM +0530, Gautam Ramakrishnan wrote:
> That was an extremely careless error. Apologies for that. Shall fix
> that immediately. However, is changing the return type of get_tlm()
> the right way?

its probably a good idea to change the return type to int


thanks

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

Those who are best at talking, realize last or never when they are wrong.


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

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

Re: [FFmpeg-devel] [PATCH v1 1/4] avcodec/xsubdec: replace data_size with got_sub_ptr for better readability

2020-03-25 Thread Michael Niedermayer
On Tue, Mar 24, 2020 at 07:05:16PM +0800, lance.lmw...@gmail.com wrote:
> From: Limin Wang 
> 
> Signed-off-by: Limin Wang 
> ---
>  libavcodec/xsubdec.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

will apply patchset

thx

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

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



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

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

Re: [FFmpeg-devel] [PATCH v2] avformat/matroskaenc: Write level 1 elements in one go

2020-03-25 Thread Andreas Rheinhardt
Carl Eugen Hoyos:
> Am Mi., 25. März 2020 um 05:45 Uhr schrieb Andreas Rheinhardt
> :
>>
>> Up until now, writing level 1 elements proceeded as follows: First, the
>> element id was written to the ordinary output AVIOContext and a dynamic
>> buffer was opened for the content of the level 1 element in
>> start_ebml_master_crc32(). Then this buffer was actually used and after it
>> was closed (in end_ebml_master_crc32()), the size field corresponding to
>> the buffer's size was written, after which the actual data was written.
>>
>> This commit changes this: Nothing is written to the main AVIOContext any
>> more in start_ebml_master_crc32(). end_ebml_master_crc32() now writes
>> both the id, the length field as well as the data. This implies that
>> one can start a level 1 element in memory without outputting anything.
>> This is done to enable to test whether enough space has been reserved
>> for the Cues (if space has been reserved for them) before writing them.
>> A large duration between outputting the header and outputting the rest
>> could also break certain streaming usecases like the one from #8578
>> (which this commit fixes).
>>
>> Signed-off-by: Andreas Rheinhardt 
>> ---
>> The commit message has been updated in light of ticket #8578; nothing
>> else has changed.
> 
> Please mention the ticket in the commit message.
> 
It is already contained in the commit message.

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

Re: [FFmpeg-devel] [PATCH v2] avformat/matroskaenc: Write level 1 elements in one go

2020-03-25 Thread James Almer
On 3/25/2020 1:44 AM, Andreas Rheinhardt wrote:
> Up until now, writing level 1 elements proceeded as follows: First, the
> element id was written to the ordinary output AVIOContext and a dynamic
> buffer was opened for the content of the level 1 element in
> start_ebml_master_crc32(). Then this buffer was actually used and after it
> was closed (in end_ebml_master_crc32()), the size field corresponding to
> the buffer's size was written, after which the actual data was written.
> 
> This commit changes this: Nothing is written to the main AVIOContext any
> more in start_ebml_master_crc32(). end_ebml_master_crc32() now writes
> both the id, the length field as well as the data. This implies that
> one can start a level 1 element in memory without outputting anything.
> This is done to enable to test whether enough space has been reserved
> for the Cues (if space has been reserved for them) before writing them.
> A large duration between outputting the header and outputting the rest
> could also break certain streaming usecases like the one from #8578
> (which this commit fixes).
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
> The commit message has been updated in light of ticket #8578; nothing
> else has changed.

Should be ok. And please backport it to release/4.2 branch using git
cherry-pick -x, so the commit hash from master is referenced.

> 
> I'd like to apply this and everything until (but excluding) #18 [1]
> in the coming days to fix the aforementioned issue (I will also backport
> a fix to 4.2).
> 
> [1] is the cutoff point because upon rereading I am not sure anymore
> whether it is good to write the cues at the end if the reserved space
> doesn't suffice and return normally afterwards, because the fact that
> the cues have not been written at the front won't be detectable from the
> return value of av_write_trailer() any more (one only gets a
> AV_LOG_WARNING logmessage).
> 
> [1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2020-January/255157.html
> 
>  libavformat/matroskaenc.c | 60 +--
>  1 file changed, 32 insertions(+), 28 deletions(-)
> 
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 66f1bc4255..501d68a8f7 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -325,26 +325,26 @@ static void end_ebml_master(AVIOContext *pb, 
> ebml_master master)
>  avio_seek(pb, pos, SEEK_SET);
>  }
>  
> -static int start_ebml_master_crc32(AVIOContext *pb, AVIOContext **dyn_cp, 
> MatroskaMuxContext *mkv,
> -   uint32_t elementid)
> +static int start_ebml_master_crc32(AVIOContext **dyn_cp, MatroskaMuxContext 
> *mkv)
>  {
>  int ret;
>  
>  if ((ret = avio_open_dyn_buf(dyn_cp)) < 0)
>  return ret;
>  
> -put_ebml_id(pb, elementid);
>  if (mkv->write_crc)
>  put_ebml_void(*dyn_cp, 6); /* Reserve space for CRC32 so 
> position/size calculations using avio_tell() take it into account */
>  
>  return 0;
>  }
>  
> -static void end_ebml_master_crc32(AVIOContext *pb, AVIOContext **dyn_cp, 
> MatroskaMuxContext *mkv)
> +static void end_ebml_master_crc32(AVIOContext *pb, AVIOContext **dyn_cp,
> +  MatroskaMuxContext *mkv, uint32_t id)
>  {
>  uint8_t *buf, crc[4];
>  int size, skip = 0;
>  
> +put_ebml_id(pb, id);
>  size = avio_close_dyn_buf(*dyn_cp, );
>  put_ebml_num(pb, size, 0);
>  if (mkv->write_crc) {
> @@ -362,13 +362,14 @@ static void end_ebml_master_crc32(AVIOContext *pb, 
> AVIOContext **dyn_cp, Matrosk
>  * Complete ebml master without destroying the buffer, allowing for later 
> updates
>  */
>  static void end_ebml_master_crc32_preliminary(AVIOContext *pb, AVIOContext 
> **dyn_cp, MatroskaMuxContext *mkv,
> -  int64_t *pos)
> +  uint32_t id, int64_t *pos)
>  {
>  uint8_t *buf;
>  int size = avio_get_dyn_buf(*dyn_cp, );
>  
>  *pos = avio_tell(pb);
>  
> +put_ebml_id(pb, id);
>  put_ebml_num(pb, size, 0);
>  avio_write(pb, buf, size);
>  }
> @@ -450,7 +451,7 @@ static int mkv_write_seekhead(AVIOContext *pb, 
> MatroskaMuxContext *mkv,
>  goto seek;
>  }
>  
> -ret = start_ebml_master_crc32(pb, _cp, mkv, MATROSKA_ID_SEEKHEAD);
> +ret = start_ebml_master_crc32(_cp, mkv);
>  if (ret < 0)
>  return ret;
>  
> @@ -466,7 +467,7 @@ static int mkv_write_seekhead(AVIOContext *pb, 
> MatroskaMuxContext *mkv,
>  put_ebml_uint(dyn_cp, MATROSKA_ID_SEEKPOSITION, entry->segmentpos);
>  end_ebml_master(dyn_cp, seekentry);
>  }
> -end_ebml_master_crc32(pb, _cp, mkv);
> +end_ebml_master_crc32(pb, _cp, mkv, MATROSKA_ID_SEEKHEAD);
>  
>  remaining = seekhead->filepos + seekhead->reserved_size - avio_tell(pb);
>  put_ebml_void(pb, remaining);
> @@ -510,7 +511,7 @@ static int64_t mkv_write_cues(AVFormatContext *s, 
> mkv_cues 

Re: [FFmpeg-devel] [PATCH v2] avformat/matroskaenc: Write level 1 elements in one go

2020-03-25 Thread Carl Eugen Hoyos
Am Mi., 25. März 2020 um 05:45 Uhr schrieb Andreas Rheinhardt
:
>
> Up until now, writing level 1 elements proceeded as follows: First, the
> element id was written to the ordinary output AVIOContext and a dynamic
> buffer was opened for the content of the level 1 element in
> start_ebml_master_crc32(). Then this buffer was actually used and after it
> was closed (in end_ebml_master_crc32()), the size field corresponding to
> the buffer's size was written, after which the actual data was written.
>
> This commit changes this: Nothing is written to the main AVIOContext any
> more in start_ebml_master_crc32(). end_ebml_master_crc32() now writes
> both the id, the length field as well as the data. This implies that
> one can start a level 1 element in memory without outputting anything.
> This is done to enable to test whether enough space has been reserved
> for the Cues (if space has been reserved for them) before writing them.
> A large duration between outputting the header and outputting the rest
> could also break certain streaming usecases like the one from #8578
> (which this commit fixes).
>
> Signed-off-by: Andreas Rheinhardt 
> ---
> The commit message has been updated in light of ticket #8578; nothing
> else has changed.

Please mention the ticket in the commit message.

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] [PATCH] libavcodec/jpeg2000.h: fix comments for JPEG2000 markers

2020-03-25 Thread gautamramk
From: Gautam Ramakrishnan 

The comments for some of the markers were incorrect.
This patch fixes the comments associated with the markers.
---
 libavcodec/jpeg2000.h | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/libavcodec/jpeg2000.h b/libavcodec/jpeg2000.h
index c429ca5996..7b78c0193e 100644
--- a/libavcodec/jpeg2000.h
+++ b/libavcodec/jpeg2000.h
@@ -40,15 +40,15 @@ enum Jpeg2000Markers {
 JPEG2000_SIZ = 0xff51, // image and tile size
 JPEG2000_COD,  // coding style default
 JPEG2000_COC,  // coding style component
-JPEG2000_TLM = 0xff55, // packed packet headers, tile-part header
-JPEG2000_PLM = 0xff57, // tile-part lengths
-JPEG2000_PLT,  // packet length, main header
+JPEG2000_TLM = 0xff55, // tile-part length, main header
+JPEG2000_PLM = 0xff57, // packet length, main header
+JPEG2000_PLT,  // packet length, tile-part header
 JPEG2000_QCD = 0xff5c, // quantization default
 JPEG2000_QCC,  // quantization component
 JPEG2000_RGN,  // region of interest
 JPEG2000_POC,  // progression order change
-JPEG2000_PPM,  // packet length, tile-part header
-JPEG2000_PPT,  // packed packet headers, main header
+JPEG2000_PPM,  // packed packet headers, main header
+JPEG2000_PPT,  // packed packet headers, tile-part header
 JPEG2000_CRG = 0xff63, // component registration
 JPEG2000_COM,  // comment
 JPEG2000_SOT = 0xff90, // start of tile-part
-- 
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 v1 2/2] avfilter/vf_showinfo: limit the max number of timecode

2020-03-25 Thread lance . lmwang
From: Limin Wang 

Signed-off-by: Limin Wang 
---
 libavfilter/vf_showinfo.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c
index 71534d1fa8..b0b0051357 100644
--- a/libavfilter/vf_showinfo.c
+++ b/libavfilter/vf_showinfo.c
@@ -318,14 +318,15 @@ static int filter_frame(AVFilterLink *inlink, AVFrame 
*frame)
 break;
 case AV_FRAME_DATA_S12M_TIMECODE: {
 uint32_t *tc = (uint32_t*)sd->data;
+int m = FFMIN(tc[0],3);
 if (sd->size != 16) {
 av_log(ctx, AV_LOG_ERROR, "invalid data");
 break;
 }
-for (int j = 1; j <= tc[0]; j++) {
+for (int j = 1; j <= m; j++) {
 char tcbuf[AV_TIMECODE_STR_SIZE];
 av_timecode_make_smpte_tc_string(tcbuf, tc[j], 0);
-av_log(ctx, AV_LOG_INFO, "timecode - %s%s", tcbuf, j != tc[0] 
? ", " : "");
+av_log(ctx, AV_LOG_INFO, "timecode - %s%s", tcbuf, j != m ? ", 
" : "");
 }
 break;
 }
-- 
2.21.0

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

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

[FFmpeg-devel] [PATCH v1 1/2] avfilter/vf_showinfo: check if the s12m data size is valid

2020-03-25 Thread lance . lmwang
From: Limin Wang 

Signed-off-by: Limin Wang 
---
 libavfilter/vf_showinfo.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c
index 79b79db2d3..71534d1fa8 100644
--- a/libavfilter/vf_showinfo.c
+++ b/libavfilter/vf_showinfo.c
@@ -318,6 +318,10 @@ static int filter_frame(AVFilterLink *inlink, AVFrame 
*frame)
 break;
 case AV_FRAME_DATA_S12M_TIMECODE: {
 uint32_t *tc = (uint32_t*)sd->data;
+if (sd->size != 16) {
+av_log(ctx, AV_LOG_ERROR, "invalid data");
+break;
+}
 for (int j = 1; j <= tc[0]; j++) {
 char tcbuf[AV_TIMECODE_STR_SIZE];
 av_timecode_make_smpte_tc_string(tcbuf, tc[j], 0);
-- 
2.21.0

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

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

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

2020-03-25 Thread Pestov Vyacheslav
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".

Re: [FFmpeg-devel] [PATCH] avcodec/decode: Clear format on ff_get_buffer() failure

2020-03-25 Thread Anton Khirnov
Quoting Michael Niedermayer (2020-03-24 21:23:58)
> On Tue, Mar 24, 2020 at 10:59:21AM +0100, Anton Khirnov wrote:
> > Quoting Michael Niedermayer (2020-03-24 01:41:43)
> > > Fixes: out of array access
> > > Fixes: 
> > > 21193/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WAVPACK_fuzzer-5125168956702720
> > > 
> > > Found-by: continuous fuzzing process 
> > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > Signed-off-by: Michael Niedermayer 
> > > ---
> > >  libavcodec/decode.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> > > index af6bb3f952..fb22ef0ef3 100644
> > > --- a/libavcodec/decode.c
> > > +++ b/libavcodec/decode.c
> > > @@ -1972,6 +1972,7 @@ int ff_get_buffer(AVCodecContext *avctx, AVFrame 
> > > *frame, int flags)
> > >  if (ret < 0) {
> > >  av_log(avctx, AV_LOG_ERROR, "get_buffer() failed\n");
> > >  frame->width = frame->height = 0;
> > > +frame->format = -1;
> > 
> > This looks ad-hoc. And also unnecessary, since get_buffer_internal() is
> > already calling av_frame_unref() on failure.
> > Seems it's just missing the cleanup goto on ff_decode_frame_props()
> > failure.
> 
> will post a patch to fix that cleanup but i have to say that i 
> feel a bit uneasy that a fix to out of array access is "not forgetting
> cleanup on any path"

As I see it, we should not be fixing a segfault/invalid access. That
segfault is not in itself a bug, merely a symptom of one. The actual bug
is that the program got into an invalid state, which we fix by making
the code fail in a safe way.

I believe that approach is better than sprinkling random checks in
various places that only detect some limited instances of invalid state,
but do not prevent it from happening in the first place.

> Ill add a assert so if another path leads to this, it at least gets
> stoped/clearly caught.

I think it might be clearest to merge ff_get_buffer() and
get_buffer_internal(). Since the purpose of separation is just to print
an error message on failure and get_buffer_internal() already contains a
cleanup block, we can just make all failed paths go through cleanup.
That will make it obvious that the frame is always clean on failure and
prevent not just this crash but any other crashes possibly arising from
the frame not being clean.

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

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