Re: [FFmpeg-devel] [PATCH] Gsoc: add the two fuzzy targets

2021-04-22 Thread Heng Zhang 


> 在 2021年4月22日，下午6:53，Michael Niedermayer  写道：
> 
> On Thu, Apr 22, 2021 at 04:13:56PM +0800, Heng Zhang wrote:
>> 
>> 
>>> 在 2021年4月20日，下午7:12，Michael Niedermayer  写道：
>>> 
>>> On Tue, Apr 20, 2021 at 12:34:13PM +0800, Heng Zhang wrote:
 
 
> 在 2021年4月19日，下午5:47，Michael Niedermayer  写道：
> 
> On Mon, Apr 19, 2021 at 05:06:10PM +0800, a397341...@163.com 
>  wrote:
>> From: toseven 
>>> [...]
>> +if (ret < 0)
>> +{
>> +fprintf(stderr, "Error occurred in av_packet_add_side_data: 
>> %s\n",
>> +av_err2str(ret));
>> +}
>> +return ret;
> 
> the { } placing style mismatches whats used in FFmpeg (i dont mind but 
> some people do mind)
> 
> more general, how much code coverage is gained with these 2 fuzzers 
> compared to what already exists ?
> 
> thanks
 
 Okay, I will modify my style to adopt for FFmpeg. What is more, I didn’t 
 compare the code coverage between them. Do I have to do this?  I mainly 
 refer to the fate test from libavcodec/tests/avpacket.c and 
 libavfilter/tests/formats.c.
>>> 
>>> If code coverage does not improve, what would be the reason for FFmpeg to
>>> include the code ?
>> 
>> Thank your reply. 
>> My fuzzing targets call the new API interfaces, which are not used by the 
>> existing fuzzing target. Though I don’t do the related experiment, code 
>> coverage should improve. 
> 
> Current fuzzer coverage can be seen here:
> https://storage.googleapis.com/oss-fuzz-coverage/ffmpeg/reports/20210420/linux/src/ffmpeg/report.html
>  
> 
> 
> You are adding 2 targets
> target_avpacket_fuzzer:
> this calls
> av_packet_side_data_name, av_packet_add_side_data, av_packet_free, 
> av_packet_clone, av_grow_packet, av_new_packet, av_packet_from_data, 
> From these all but av_packet_clone and av_packet_side_data_name are covered 
> already
> av_packet_side_data_name() is called with a fixed argument in your code
> 
> target_formats_fuzzer:
> this calls av_get_channel_layout_string, ff_parse_channel_layout
> the first is already covered the second is in libavfilter
> libavfilter needs to be fuzzed, such fuzzing would involve building
> filter chains or networks based on fuzzer input.
> A 2nd set of libavfilter fuzzers should similar to libavcodec fuzzers
> generate 1 fuzzer generically for each avfilter similarly to how decoders
> from libavcodec are fuzzed.
> Such libavfilter fuzzers would then also test most functions within 
> libavfilter
> 
> More generally about coverage.
> If you where in my position what would you want for additional fuzzers ?
> maximally increased coverage with mininmal effort ?
> I belive this would be achieved with generic fuzzing of filters similar to how
> decoders are fuzzed currently. But thats a bit bigger effort 
> 
> I see the gsoc page says connecting 2 fate tests to fuzzing can be used as
> qualification task. 
> For connecting such tests, the fate test and the fuzzer should use shared
> code and not duplicate. One way that can work is that the fate test takes
> some input and that input is fixed for fate but can change when used for 
> fuzzing
> again, the more coverage we can achieve with as little effort the better.
> Basically dont be afraid to submit a small amount of code because in fact
> i would be more impressed if you can connect fate test(s) with little code
> to the fuzzer than with alot of code.
> 
> Thanks

Thank you for your patient reply. I will carefully consider your comments and 
submit the code again according to your suggestions.
> 
> [...]
> -- 
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> While the State exists there can be no freedom; when there is freedom there
> will be no State. -- Vladimir Lenin
> ___
> 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 7/9] avformat/id3v2: Check end for overflow in id3v2_parse()

2021-04-22 Thread James Almer 

On 4/19/2021 3:23 PM, Michael Niedermayer wrote:

Fixes: signed integer overflow: 9223372036840103978 + 67637280 cannot be 
represented in type 'long'
Fixes: 
33341/clusterfuzz-testcase-minimized-ffmpeg_dem_DSF_fuzzer-6408154041679872

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

diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c
index e0fef08789..0f7035d4c5 100644
--- a/libavformat/id3v2.c
+++ b/libavformat/id3v2.c
@@ -824,7 +824,7 @@ static void id3v2_parse(AVIOContext *pb, AVDictionary 
**metadata,
  int isv34, unsync;
  unsigned tlen;
  char tag[5];
-int64_t next, end = avio_tell(pb) + len;
+int64_t next, end = avio_tell(pb);
  int taghdrlen;
  const char *reason = NULL;
  AVIOContext pb_local;
@@ -836,6 +836,10 @@ static void id3v2_parse(AVIOContext *pb, AVDictionary 
**metadata,
  av_unused int uncompressed_buffer_size = 0;
  const char *comm_frame;
  
+if (av_sat_add64(end, len) != end + (uint64_t)len)


Wouldn't a check like end > INT64_MAX - len be simpler?


+return;
+end += len;
+
  av_log(s, AV_LOG_DEBUG, "id3v2 ver:%d flags:%02X len:%d\n", version, 
flags, len);
  
  switch (version) {

@@ -1057,7 +1061,7 @@ seek:
  
  /* Footer preset, always 10 bytes, skip over it */

  if (version == 4 && flags & 0x10)
-end += 10;
+end = av_sat_add64(end, 10);
  
  error:

  if (reason)



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

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

[FFmpeg-devel] [PATCH] avcodec/av1_metadata: don't store the inserted TD OBU in stack

2021-04-22 Thread James Almer 
Fixes: stack-use-after-return
Fixes: 
clusterfuzz-testcase-minimized-ffmpeg_BSF_AV1_METADATA_fuzzer-5931515701755904
Fixes: 
clusterfuzz-testcase-minimized-ffmpeg_BSF_AV1_METADATA_fuzzer-6105676541722624

Signed-off-by: James Almer 
---
 libavcodec/av1_metadata_bsf.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/libavcodec/av1_metadata_bsf.c b/libavcodec/av1_metadata_bsf.c
index 328db5c0da..b1ae364431 100644
--- a/libavcodec/av1_metadata_bsf.c
+++ b/libavcodec/av1_metadata_bsf.c
@@ -28,6 +28,7 @@ typedef struct AV1MetadataContext {
 CBSBSFContext common;
 
 int td;
+AV1RawOBU td_obu;
 
 int color_primaries;
 int transfer_characteristics;
@@ -107,7 +108,7 @@ static int av1_metadata_update_fragment(AVBSFContext *bsf, 
AVPacket *pkt,
 CodedBitstreamFragment *frag)
 {
 AV1MetadataContext *ctx = bsf->priv_data;
-AV1RawOBU td, *obu;
+AV1RawOBU *obu;
 int err, i;
 
 for (i = 0; i < frag->nb_units; i++) {
@@ -124,12 +125,12 @@ static int av1_metadata_update_fragment(AVBSFContext 
*bsf, AVPacket *pkt,
 if (ctx->td == BSF_ELEMENT_REMOVE)
 ff_cbs_delete_unit(frag, 0);
 } else if (pkt && ctx->td == BSF_ELEMENT_INSERT) {
-td = (AV1RawOBU) {
+ctx->td_obu = (AV1RawOBU) {
 .header.obu_type = AV1_OBU_TEMPORAL_DELIMITER,
 };
 
 err = ff_cbs_insert_unit_content(frag, 0, AV1_OBU_TEMPORAL_DELIMITER,
- &td, NULL);
+ &ctx->td_obu, NULL);
 if (err < 0) {
 av_log(bsf, AV_LOG_ERROR, "Failed to insert Temporal 
Delimiter.\n");
 return err;
-- 
2.31.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 1/7] avcodec/avcodec: Actually honour the documentation of subtitle_header

2021-04-22 Thread Andreas Rheinhardt 
James Almer:
> On 4/22/2021 2:34 PM, Andreas Rheinhardt wrote:
>> James Almer:
>>> On 4/18/2021 11:00 PM, Andreas Rheinhardt wrote:
 It is only supposed to be freed by libavcodec for decoders, yet
 avcodec_open2() always frees it on failure.
 Furthermore, avcodec_close() doesn't free it for decoders.
 Both of this has been changed.

 Signed-off-by: Andreas Rheinhardt 
 ---
 This might be squashed with the next patch.

    libavcodec/avcodec.c | 7 +--
    1 file changed, 5 insertions(+), 2 deletions(-)

 diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
 index 760a98d8ef..24f6922d4f 100644
 --- a/libavcodec/avcodec.c
 +++ b/libavcodec/avcodec.c
 @@ -418,7 +418,8 @@ FF_ENABLE_DEPRECATION_WARNINGS
      av_dict_free(&tmp);
    av_freep(&avctx->priv_data);
 -    av_freep(&avctx->subtitle_header);
 +    if (av_codec_is_decoder(avctx->codec))
 +    av_freep(&avctx->subtitle_header);
      #if FF_API_OLD_ENCDEC
    av_frame_free(&avci->to_free);
 @@ -589,7 +590,9 @@ FF_DISABLE_DEPRECATION_WARNINGS
    av_frame_free(&avctx->coded_frame);
    FF_ENABLE_DEPRECATION_WARNINGS
    #endif
 -    }
 +    } else if (av_codec_is_decoder(avctx->codec))
 +    av_freep(&avctx->subtitle_header);
 +
    avctx->codec = NULL;
    avctx->active_thread_type = 0;
>>>
>>> LGTM. Same should be done for extradata, it seems.
>>
>> You mean adding documentation that avcodec_free_context() frees it
>> despite the documentation saying that this doesn't happen for decoders?
> 
> No, i meant adapting the code to follow the doxy.
> 
Adapting avcodec_free_context() (avcodec_close() and avcodec_open2()
already honour the doxy) would probably break lots of code (i.e.
introduce leaks), so I'd rather opt for adapting the doxy.

- 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 1/7] avcodec/avcodec: Actually honour the documentation of subtitle_header

2021-04-22 Thread James Almer 

On 4/22/2021 2:34 PM, Andreas Rheinhardt wrote:

James Almer:

On 4/18/2021 11:00 PM, Andreas Rheinhardt wrote:

It is only supposed to be freed by libavcodec for decoders, yet
avcodec_open2() always frees it on failure.
Furthermore, avcodec_close() doesn't free it for decoders.
Both of this has been changed.

Signed-off-by: Andreas Rheinhardt 
---
This might be squashed with the next patch.

   libavcodec/avcodec.c | 7 +--
   1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
index 760a98d8ef..24f6922d4f 100644
--- a/libavcodec/avcodec.c
+++ b/libavcodec/avcodec.c
@@ -418,7 +418,8 @@ FF_ENABLE_DEPRECATION_WARNINGS
     av_dict_free(&tmp);
   av_freep(&avctx->priv_data);
-    av_freep(&avctx->subtitle_header);
+    if (av_codec_is_decoder(avctx->codec))
+    av_freep(&avctx->subtitle_header);
     #if FF_API_OLD_ENCDEC
   av_frame_free(&avci->to_free);
@@ -589,7 +590,9 @@ FF_DISABLE_DEPRECATION_WARNINGS
   av_frame_free(&avctx->coded_frame);
   FF_ENABLE_DEPRECATION_WARNINGS
   #endif
-    }
+    } else if (av_codec_is_decoder(avctx->codec))
+    av_freep(&avctx->subtitle_header);
+
   avctx->codec = NULL;
   avctx->active_thread_type = 0;


LGTM. Same should be done for extradata, it seems.


You mean adding documentation that avcodec_free_context() frees it
despite the documentation saying that this doesn't happen for decoders?


No, i meant adapting the code to follow the doxy.



- 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 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/7] avcodec/avcodec: Actually honour the documentation of subtitle_header

2021-04-22 Thread Andreas Rheinhardt 
James Almer:
> On 4/18/2021 11:00 PM, Andreas Rheinhardt wrote:
>> It is only supposed to be freed by libavcodec for decoders, yet
>> avcodec_open2() always frees it on failure.
>> Furthermore, avcodec_close() doesn't free it for decoders.
>> Both of this has been changed.
>>
>> Signed-off-by: Andreas Rheinhardt 
>> ---
>> This might be squashed with the next patch.
>>
>>   libavcodec/avcodec.c | 7 +--
>>   1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
>> index 760a98d8ef..24f6922d4f 100644
>> --- a/libavcodec/avcodec.c
>> +++ b/libavcodec/avcodec.c
>> @@ -418,7 +418,8 @@ FF_ENABLE_DEPRECATION_WARNINGS
>>     av_dict_free(&tmp);
>>   av_freep(&avctx->priv_data);
>> -    av_freep(&avctx->subtitle_header);
>> +    if (av_codec_is_decoder(avctx->codec))
>> +    av_freep(&avctx->subtitle_header);
>>     #if FF_API_OLD_ENCDEC
>>   av_frame_free(&avci->to_free);
>> @@ -589,7 +590,9 @@ FF_DISABLE_DEPRECATION_WARNINGS
>>   av_frame_free(&avctx->coded_frame);
>>   FF_ENABLE_DEPRECATION_WARNINGS
>>   #endif
>> -    }
>> +    } else if (av_codec_is_decoder(avctx->codec))
>> +    av_freep(&avctx->subtitle_header);
>> +
>>   avctx->codec = NULL;
>>   avctx->active_thread_type = 0;
> 
> LGTM. Same should be done for extradata, it seems.

You mean adding documentation that avcodec_free_context() frees it
despite the documentation saying that this doesn't happen for decoders?

- 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 1/7] avcodec/avcodec: Actually honour the documentation of subtitle_header

2021-04-22 Thread James Almer 

On 4/18/2021 11:00 PM, Andreas Rheinhardt wrote:

It is only supposed to be freed by libavcodec for decoders, yet
avcodec_open2() always frees it on failure.
Furthermore, avcodec_close() doesn't free it for decoders.
Both of this has been changed.

Signed-off-by: Andreas Rheinhardt 
---
This might be squashed with the next patch.

  libavcodec/avcodec.c | 7 +--
  1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
index 760a98d8ef..24f6922d4f 100644
--- a/libavcodec/avcodec.c
+++ b/libavcodec/avcodec.c
@@ -418,7 +418,8 @@ FF_ENABLE_DEPRECATION_WARNINGS
  
  av_dict_free(&tmp);

  av_freep(&avctx->priv_data);
-av_freep(&avctx->subtitle_header);
+if (av_codec_is_decoder(avctx->codec))
+av_freep(&avctx->subtitle_header);
  
  #if FF_API_OLD_ENCDEC

  av_frame_free(&avci->to_free);
@@ -589,7 +590,9 @@ FF_DISABLE_DEPRECATION_WARNINGS
  av_frame_free(&avctx->coded_frame);
  FF_ENABLE_DEPRECATION_WARNINGS
  #endif
-}
+} else if (av_codec_is_decoder(avctx->codec))
+av_freep(&avctx->subtitle_header);
+
  avctx->codec = NULL;
  avctx->active_thread_type = 0;


LGTM. Same should be done for extradata, it seems.
___
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/7] avcodec/avcodec: Actually honour the documentation of subtitle_header

2021-04-22 Thread Andreas Rheinhardt 
Andreas Rheinhardt:
> It is only supposed to be freed by libavcodec for decoders, yet
> avcodec_open2() always frees it on failure.
> Furthermore, avcodec_close() doesn't free it for decoders.
> Both of this has been changed.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
> This might be squashed with the next patch.
> 
>  libavcodec/avcodec.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
> index 760a98d8ef..24f6922d4f 100644
> --- a/libavcodec/avcodec.c
> +++ b/libavcodec/avcodec.c
> @@ -418,7 +418,8 @@ FF_ENABLE_DEPRECATION_WARNINGS
>  
>  av_dict_free(&tmp);
>  av_freep(&avctx->priv_data);
> -av_freep(&avctx->subtitle_header);
> +if (av_codec_is_decoder(avctx->codec))
> +av_freep(&avctx->subtitle_header);
>  
>  #if FF_API_OLD_ENCDEC
>  av_frame_free(&avci->to_free);
> @@ -589,7 +590,9 @@ FF_DISABLE_DEPRECATION_WARNINGS
>  av_frame_free(&avctx->coded_frame);
>  FF_ENABLE_DEPRECATION_WARNINGS
>  #endif
> -}
> +} else if (av_codec_is_decoder(avctx->codec))
> +av_freep(&avctx->subtitle_header);
> +
>  avctx->codec = NULL;
>  avctx->active_thread_type = 0;
>  
> 
Ping for this patchset.

- 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 2/2 v2] avcodec/mjpegdec: postpone calling ff_get_buffer() until the SOS marker

2021-04-22 Thread James Almer 
With JPEG-LS PAL8 samples, the JPEG-LS extension parameters signaled with
the LSE marker show up after SOF but before SOS. For those, the pixel format
chosen by get_format() in SOF is GRAY8, and then replaced by PAL8 in LSE.
This has not been an issue given both pixel formats allocate the second data
plane for the palette, but after the upcoming soname bump, GRAY8 will no longer
do that. This will result in segfauls when ff_jpegls_decode_lse() attempts to
write the palette on a buffer originally allocated as a GRAY8 one.

Work around this by calling ff_get_buffer() after the actual pixel format is
known.

Signed-off-by: James Almer 
---
Now splitting the variable that signals that a frame was allocated and that
SOF data was successfully parsed.

 libavcodec/jpeglsdec.c |  6 +--
 libavcodec/mjpegbdec.c |  1 +
 libavcodec/mjpegdec.c  | 83 +-
 libavcodec/mjpegdec.h  |  3 ++
 libavcodec/mxpegdec.c  |  2 +-
 5 files changed, 56 insertions(+), 39 deletions(-)

diff --git a/libavcodec/jpeglsdec.c b/libavcodec/jpeglsdec.c
index e17de09e9f..4ab0cb5bd8 100644
--- a/libavcodec/jpeglsdec.c
+++ b/libavcodec/jpeglsdec.c
@@ -108,9 +108,8 @@ int ff_jpegls_decode_lse(MJpegDecodeContext *s)
 if (s->palette_index > maxtab)
 return AVERROR_INVALIDDATA;
 
-if ((s->avctx->pix_fmt == AV_PIX_FMT_GRAY8 || s->avctx->pix_fmt == 
AV_PIX_FMT_PAL8) &&
-(s->picture_ptr->format == AV_PIX_FMT_GRAY8 || 
s->picture_ptr->format == AV_PIX_FMT_PAL8)) {
-uint32_t *pal = (uint32_t *)s->picture_ptr->data[1];
+if (s->avctx->pix_fmt == AV_PIX_FMT_GRAY8 || s->avctx->pix_fmt == 
AV_PIX_FMT_PAL8) {
+uint32_t *pal = s->palette;
 int shift = 0;
 
 if (s->avctx->bits_per_raw_sample > 0 && 
s->avctx->bits_per_raw_sample < 8) {
@@ -118,7 +117,6 @@ int ff_jpegls_decode_lse(MJpegDecodeContext *s)
 shift = 8 - s->avctx->bits_per_raw_sample;
 }
 
-s->picture_ptr->format =
 s->avctx->pix_fmt = AV_PIX_FMT_PAL8;
 for (i=s->palette_index; i<=maxtab; i++) {
 uint8_t k = i << shift;
diff --git a/libavcodec/mjpegbdec.c b/libavcodec/mjpegbdec.c
index 774908..890befb522 100644
--- a/libavcodec/mjpegbdec.c
+++ b/libavcodec/mjpegbdec.c
@@ -55,6 +55,7 @@ static int mjpegb_decode_frame(AVCodecContext *avctx,
 
 buf_ptr = buf;
 buf_end = buf + buf_size;
+s->seen_sof = 0;
 s->got_picture = 0;
 s->adobe_transform = -1;
 
diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
index 7c7cc20af8..42e6170469 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -138,6 +138,7 @@ av_cold int ff_mjpeg_decode_init(AVCodecContext *avctx)
 s->buffer= NULL;
 s->start_code= -1;
 s->first_picture = 1;
+s->seen_sof  = 0;
 s->got_picture   = 0;
 s->orig_height= avctx->coded_height;
 avctx->chroma_sample_location = AVCHROMA_LOC_CENTER;
@@ -429,6 +430,7 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s)
 memcpy(s->h_count, h_count, sizeof(h_count));
 memcpy(s->v_count, v_count, sizeof(v_count));
 s->interlaced = 0;
+s->seen_sof = 0;
 s->got_picture = 0;
 
 /* test interlaced mode */
@@ -681,11 +683,13 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s)
 } else if (s->nb_components != 1) {
 av_log(s->avctx, AV_LOG_ERROR, "Unsupported number of 
components %d\n", s->nb_components);
 return AVERROR_PATCHWELCOME;
-} else if (s->palette_index && s->bits <= 8)
-s->avctx->pix_fmt = AV_PIX_FMT_PAL8;
-else if (s->bits <= 8)
-s->avctx->pix_fmt = AV_PIX_FMT_GRAY8;
-else
+} else if (s->bits <= 8) {
+avpriv_set_systematic_pal2(s->palette, s->avctx->pix_fmt);
+if (s->palette_index)
+s->avctx->pix_fmt = AV_PIX_FMT_PAL8;
+else
+s->avctx->pix_fmt = AV_PIX_FMT_GRAY8;
+} else
 s->avctx->pix_fmt = AV_PIX_FMT_GRAY16;
 }
 
@@ -719,26 +723,13 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s)
 if (s->avctx->skip_frame == AVDISCARD_ALL) {
 s->picture_ptr->pict_type = AV_PICTURE_TYPE_I;
 s->picture_ptr->key_frame = 1;
-s->got_picture= 1;
+s->seen_sof   = 1;
 return 0;
 }
-
-av_frame_unref(s->picture_ptr);
-if (ff_get_buffer(s->avctx, s->picture_ptr, AV_GET_BUFFER_FLAG_REF) < 
0)
-return -1;
-s->picture_ptr->pict_type = AV_PICTURE_TYPE_I;
-s->picture_ptr->key_frame = 1;
-s->got_picture= 1;
-
-for (i = 0; i < 4; i++)
-s->linesize[i] = s->picture_ptr->linesize[i] << s->interlaced;
-
-ff_dlog(s->avctx, "%d %d %d %d %d %d\n",
-s->widt

Re: [FFmpeg-devel] [PATCH 4/9] avformat/wtvdec: Improve size overflow checks in parse_chunks()

2021-04-22 Thread Michael Niedermayer 
On Wed, Apr 21, 2021 at 06:28:41PM +1000, Peter Ross wrote:
> On Mon, Apr 19, 2021 at 08:23:41PM +0200, Michael Niedermayer wrote:
> > Fixes: signed integer overflow: 32 + 2147483647 cannot be represented in 
> > type 'int
> > Fixes: 
> > 32967/clusterfuzz-testcase-minimized-ffmpeg_dem_WTV_fuzzer-5132856218222592
> > 
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavformat/wtvdec.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavformat/wtvdec.c b/libavformat/wtvdec.c
> > index 44ca86d517..2f1b192cea 100644
> > --- a/libavformat/wtvdec.c
> > +++ b/libavformat/wtvdec.c
> > @@ -809,7 +809,7 @@ static int parse_chunks(AVFormatContext *s, int mode, 
> > int64_t seekts, int *len_p
> >  avio_skip(pb, 12);
> >  ff_get_guid(pb, &formattype);
> >  size = avio_rl32(pb);
> > -if (size < 0 || size > INT_MAX - 92)
> > +if (size < 0 || size > INT_MAX - 92 - consumed)
> >  return AVERROR_INVALIDDATA;
> >  parse_media_type(s, 0, sid, mediatype, subtype, 
> > formattype, size);
> >  consumed += 92 + size;
> > @@ -825,7 +825,7 @@ static int parse_chunks(AVFormatContext *s, int mode, 
> > int64_t seekts, int *len_p
> >  avio_skip(pb, 12);
> >  ff_get_guid(pb, &formattype);
> >  size = avio_rl32(pb);
> > -if (size < 0 || size > INT_MAX - 76)
> > +if (size < 0 || size > INT_MAX - 76 - consumed)
> >  return AVERROR_INVALIDDATA;
> >  parse_media_type(s, s->streams[stream_index], sid, 
> > mediatype, subtype, formattype, size);
> >  consumed += 76 + size;
> > -- 
> 
> please apply

will apply

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The smallest minority on earth is the individual. Those who deny 
individual rights cannot claim to be defenders of minorities. - Ayn Rand


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] avcodec/faxcompr: Check remaining bits on error in decode_group3_1d_line()

2021-04-22 Thread Michael Niedermayer 
On Thu, Apr 15, 2021 at 10:44:20PM +0200, Michael Niedermayer wrote:
> Fixes: Timeout
> Fixes: 
> 32886/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_TIFF_fuzzer-4779761466474496
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/faxcompr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

will apply

[...]
-- 
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 2/4] tools/target_dec_fuzzer: Adjust threshold for paf video

2021-04-22 Thread Michael Niedermayer 
On Thu, Apr 15, 2021 at 10:44:18PM +0200, Michael Niedermayer wrote:
> Fixes: Timeout (long -> 2sec)
> Fixes: 
> 32790/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_PAF_VIDEO_fuzzer-5497584169910272
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  tools/target_dec_fuzzer.c | 1 +
>  1 file changed, 1 insertion(+)

will apply

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

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu


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/4] avformat/mov: check for pts overflow in mov_read_sidx()

2021-04-22 Thread Michael Niedermayer 
On Thu, Apr 15, 2021 at 10:44:17PM +0200, Michael Niedermayer wrote:
> Fixes: signed integer overflow: 9223372036846336888 + 4278255871 cannot be 
> represented in type 'long'
> Fixes: 
> 32782/clusterfuzz-testcase-minimized-ffmpeg_dem_MOV_fuzzer-6059216516284416
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavformat/mov.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

will apply

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

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 


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

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

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/mjpegdec: postpone calling ff_get_buffer() until the SOS marker

2021-04-22 Thread Michael Niedermayer 
On Wed, Apr 21, 2021 at 02:40:55PM -0300, James Almer wrote:
> With JPEG-LS PAL8 samples, the JPEG-LS extension parameters signaled with
> the LSE marker show up after SOF but before SOS. For those, the pixel format
> chosen by get_format() in SOF is GRAY8, and then replaced by PAL8 in LSE.
> This has not been an issue given both pixel formats allocate the second data
> plane for the palette, but after the upcoming soname bump, GRAY8 will no 
> longer
> do that. This will result in segfauls when ff_jpegls_decode_lse() attempts to
> write the palette on a buffer originally allocated as a GRAY8 one.
> 
> Work around this by calling ff_get_buffer() after the actual pixel format is
> known.
> 
> Signed-off-by: James Almer 
> ---
> With this, the removal of AV_PIX_FMT_FLAG_PSEUDOPAL will no longer generate
> segfauls.
> I can't test hwaccels like vaapi to ensure things still work as inteded, 
> seeing
> i also had to move the call to start_frame().
> 
> Better solutions are very much welcome.
> 
>  libavcodec/jpeglsdec.c |  6 ++--
>  libavcodec/mjpegdec.c  | 72 +++---
>  libavcodec/mjpegdec.h  |  2 ++
>  3 files changed, 43 insertions(+), 37 deletions(-)

this causes a change in one timestamp on decode of:

./ffmpeg -ss 1.2 -i ~/tickets/3245/bad-example.mov -vframes 10 outnew.avi
./ffmpeg -i out.avi -f framecrc /tmp/crc

--- /tmp/crc2021-04-22 14:56:47.981354127 +0200
+++ /tmp/crcnew 2021-04-22 14:56:32.373271448 +0200
@@ -23,8 +23,8 @@
 0,  4,  4,1,   622080, 0xf24584d3
 1,   8064,   8064, 1152, 4608, 0x70bf8dc7
 1,   9216,   9216, 1152, 4608, 0xd00f873a
+0,  5,  5,1,   622080, 0x5f8afe7d
 1,  10368,  10368, 1152, 4608, 0xd45d84b8
-0,  6,  6,1,   622080, 0x5f8afe7d
 1,  11520,  11520, 1152, 4608, 0x64b5866e
 1,  12672,  12672, 1152, 4608, 0x36a18daf
 0,  7,  7,1,   622080, 0x09918d93

 
I did not investigate why or if that is even a issue, just wanted to
report it

thx

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

The smallest minority on earth is the individual. Those who deny 
individual rights cannot claim to be defenders of minorities. - Ayn Rand


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 v3 3/3] avformat/mpegtsenc: Write stream_id into PES after stream_id decision

2021-04-22 Thread zheng qian 
Since stream_id will have effect on the existences of
PES header fields like PTS/DTS, it should be better to
guarantee stream_id variable to be identical with
exact written value.

Signed-off-by: zheng qian 
---
 libavformat/mpegtsenc.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
index 967f98931d..525c68ffd1 100644
--- a/libavformat/mpegtsenc.c
+++ b/libavformat/mpegtsenc.c
@@ -1445,28 +1445,28 @@ static void mpegts_write_pes(AVFormatContext *s, 
AVStream *st,
 is_dvb_teletext = 0;
 if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
 if (st->codecpar->codec_id == AV_CODEC_ID_DIRAC)
-*q++ = STREAM_ID_EXTENDED_STREAM_ID;
+stream_id = STREAM_ID_EXTENDED_STREAM_ID;
 else
-*q++ = STREAM_ID_VIDEO_STREAM_0;
+stream_id = STREAM_ID_VIDEO_STREAM_0;
 } else if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO &&
(st->codecpar->codec_id == AV_CODEC_ID_MP2 ||
 st->codecpar->codec_id == AV_CODEC_ID_MP3 ||
 st->codecpar->codec_id == AV_CODEC_ID_AAC)) {
-*q++ = STREAM_ID_AUDIO_STREAM_0;
+stream_id = STREAM_ID_AUDIO_STREAM_0;
 } else if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO &&
 st->codecpar->codec_id == AV_CODEC_ID_AC3 &&
 ts->m2ts_mode) {
-*q++ = STREAM_ID_EXTENDED_STREAM_ID;
+stream_id = STREAM_ID_EXTENDED_STREAM_ID;
 } else if (st->codecpar->codec_type == AVMEDIA_TYPE_DATA &&
st->codecpar->codec_id == AV_CODEC_ID_TIMED_ID3) {
-*q++ = STREAM_ID_PRIVATE_STREAM_1;
+stream_id = STREAM_ID_PRIVATE_STREAM_1;
 } else if (st->codecpar->codec_type == AVMEDIA_TYPE_DATA) {
-*q++ = stream_id != -1 ? stream_id : STREAM_ID_METADATA_STREAM;
+stream_id = stream_id != -1 ? stream_id : 
STREAM_ID_METADATA_STREAM;
 
 if (stream_id == STREAM_ID_PRIVATE_STREAM_1) /* asynchronous 
KLV */
 pts = dts = AV_NOPTS_VALUE;
 } else {
-*q++ = STREAM_ID_PRIVATE_STREAM_1;
+stream_id = STREAM_ID_PRIVATE_STREAM_1;
 if (st->codecpar->codec_type == AVMEDIA_TYPE_SUBTITLE) {
 if (st->codecpar->codec_id == AV_CODEC_ID_DVB_SUBTITLE) {
 is_dvb_subtitle = 1;
@@ -1475,6 +1475,7 @@ static void mpegts_write_pes(AVFormatContext *s, AVStream 
*st,
 }
 }
 }
+*q++ = stream_id;
 header_len = 0;
 
 if (stream_id != 0xBC &&  // program_stream_map
-- 
2.29.2

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

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

[FFmpeg-devel] [PATCH v3 2/3] avformat/mpegtsenc: Fix indentation inside if-clause in mpegts_write_pes()

2021-04-22 Thread zheng qian 
Fix indentation caused by the added stream_id judgement

Signed-off-by: zheng qian 
---
 libavformat/mpegtsenc.c | 180 
 1 file changed, 90 insertions(+), 90 deletions(-)

diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
index b59dab5174..967f98931d 100644
--- a/libavformat/mpegtsenc.c
+++ b/libavformat/mpegtsenc.c
@@ -1486,99 +1486,99 @@ static void mpegts_write_pes(AVFormatContext *s, 
AVStream *st,
 stream_id != 0xF2 &&  // DSMCC_stream
 stream_id != 0xF8) {  // ITU-T Rec. H.222.1 type E stream
 
-flags  = 0;
-if (pts != AV_NOPTS_VALUE) {
-header_len += 5;
-flags  |= 0x80;
-}
-if (dts != AV_NOPTS_VALUE && pts != AV_NOPTS_VALUE && dts != pts) {
-header_len += 5;
-flags  |= 0x40;
-}
-if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
-st->codecpar->codec_id == AV_CODEC_ID_DIRAC) {
-/* set PES_extension_flag */
-pes_extension = 1;
-flags|= 0x01;
-
-/* One byte for PES2 extension flag +
- * one byte for extension length +
- * one byte for extension id */
-header_len += 3;
-}
-/* for Blu-ray AC3 Audio the PES Extension flag should be as follow
- * otherwise it will not play sound on blu-ray
- */
-if (ts->m2ts_mode &&
-st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO &&
-st->codecpar->codec_id == AV_CODEC_ID_AC3) {
-/* set PES_extension_flag */
-pes_extension = 1;
-flags |= 0x01;
-header_len += 3;
-}
-if (is_dvb_teletext) {
-pes_header_stuffing_bytes = 0x24 - header_len;
-header_len = 0x24;
-}
-len = payload_size + header_len + 3;
-/* 3 extra bytes should be added to DVB subtitle payload: 0x20 
0x00 at the beginning and trailing 0xff */
-if (is_dvb_subtitle) {
-len += 3;
-payload_size++;
-}
-if (len > 0x)
-len = 0;
-if (ts->omit_video_pes_length && st->codecpar->codec_type == 
AVMEDIA_TYPE_VIDEO) {
-len = 0;
-}
-*q++ = len >> 8;
-*q++ = len;
-val  = 0x80;
-/* data alignment indicator is required for subtitle and data 
streams */
-if (st->codecpar->codec_type == AVMEDIA_TYPE_SUBTITLE || 
st->codecpar->codec_type == AVMEDIA_TYPE_DATA)
-val |= 0x04;
-*q++ = val;
-*q++ = flags;
-*q++ = header_len;
-if (pts != AV_NOPTS_VALUE) {
-write_pts(q, flags >> 6, pts);
-q += 5;
-}
-if (dts != AV_NOPTS_VALUE && pts != AV_NOPTS_VALUE && dts != pts) {
-write_pts(q, 1, dts);
-q += 5;
-}
-if (pes_extension && st->codecpar->codec_id == AV_CODEC_ID_DIRAC) {
-flags = 0x01;  /* set PES_extension_flag_2 */
-*q++  = flags;
-*q++  = 0x80 | 0x01; /* marker bit + extension length */
-/* Set the stream ID extension flag bit to 0 and
- * write the extended stream ID. */
-*q++ = 0x00 | 0x60;
-}
-/* For Blu-ray AC3 Audio Setting extended flags */
-if (ts->m2ts_mode &&
-pes_extension &&
-st->codecpar->codec_id == AV_CODEC_ID_AC3) {
-flags = 0x01; /* set PES_extension_flag_2 */
+flags  = 0;
+if (pts != AV_NOPTS_VALUE) {
+header_len += 5;
+flags  |= 0x80;
+}
+if (dts != AV_NOPTS_VALUE && pts != AV_NOPTS_VALUE && dts != 
pts) {
+header_len += 5;
+flags  |= 0x40;
+}
+if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
+st->codecpar->codec_id == AV_CODEC_ID_DIRAC) {
+/* set PES_extension_flag */
+pes_extension = 1;
+flags|= 0x01;
+
+/* One byte for PES2 extension flag +
+ * one byte for extension length +
+ * one byte for extension id */
+header_len += 3;
+}
+/* for Blu-ray AC3 Audio the PES Extension flag should be as 
follow
+ * otherwise it will not play sound on blu-ray
+ */
+if (ts->m2ts_mode &&
+st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO &&
+

[FFmpeg-devel] [PATCH v3 1/3] avformat/mpegtsenc: Fix mpegts_write_pes() for private_stream_2 and other types

2021-04-22 Thread zheng qian 
Changes since v2:
  Fix PES_packet_length mismatch bug

According to the PES packet definition defined in Table 2-17
of ISO_IEC_13818-1 specification, some fields like PTS/DTS or
pes_extension could only appears if the stream_id meets the
condition:

if (stream_id != 0xBC &&  // program_stream_map
stream_id != 0xBE &&  // padding_stream
stream_id != 0xBF &&  // private_stream_2
stream_id != 0xF0 &&  // ECM
stream_id != 0xF1 &&  // EMM
stream_id != 0xFF &&  // program_stream_directory
stream_id != 0xF2 &&  // DSMCC_stream
stream_id != 0xF8) // ITU-T Rec. H.222.1 type E stream

And the following stream_id types don't have fields like PTS/DTS:

else if ( stream_id == program_stream_map
|| stream_id == private_stream_2
|| stream_id == ECM
|| stream_id == EMM
|| stream_id == program_stream_directory
|| stream_id == DSMCC_stream
|| stream_id == ITU-T Rec. H.222.1 type E stream ) {
for (i = 0; i < PES_packet_length; i++) {
PES_packet_data_byte
}
}

Current implementation skipped the judgement of stream_id
causing some kind of stream like private_stream_2 to be
incorrectly written with PTS/DTS field. For example,
Japan DTV transmits news and alerts through ARIB superimpose
that utilizes private_stream_2 still could not be remuxed
correctly for now.

This patch set fixes the remuxing for private_stream_2 and
other stream_id types.

Signed-off-by: zheng qian 
---
 libavformat/mpegtsenc.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
index f302af84ff..b59dab5174 100644
--- a/libavformat/mpegtsenc.c
+++ b/libavformat/mpegtsenc.c
@@ -1476,6 +1476,16 @@ static void mpegts_write_pes(AVFormatContext *s, 
AVStream *st,
 }
 }
 header_len = 0;
+
+if (stream_id != 0xBC &&  // program_stream_map
+stream_id != 0xBE &&  // padding_stream
+stream_id != 0xBF &&  // private_stream_2
+stream_id != 0xF0 &&  // ECM
+stream_id != 0xF1 &&  // EMM
+stream_id != 0xFF &&  // program_stream_directory
+stream_id != 0xF2 &&  // DSMCC_stream
+stream_id != 0xF8) {  // ITU-T Rec. H.222.1 type E stream
+
 flags  = 0;
 if (pts != AV_NOPTS_VALUE) {
 header_len += 5;
@@ -1569,6 +1579,11 @@ static void mpegts_write_pes(AVFormatContext *s, 
AVStream *st,
 memset(q, 0xff, pes_header_stuffing_bytes);
 q += pes_header_stuffing_bytes;
 }
+} else {
+len = payload_size;
+*q++ = len >> 8;
+*q++ = len;
+}
 is_start = 0;
 }
 /* header size */
-- 
2.29.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] Gsoc: add the two fuzzy targets

2021-04-22 Thread Michael Niedermayer 
On Thu, Apr 22, 2021 at 04:13:56PM +0800, Heng Zhang wrote:
> 
> 
> > 在 2021年4月20日，下午7:12，Michael Niedermayer  写道：
> > 
> > On Tue, Apr 20, 2021 at 12:34:13PM +0800, Heng Zhang wrote:
> >> 
> >> 
> >>> 在 2021年4月19日，下午5:47，Michael Niedermayer  写道：
> >>> 
> >>> On Mon, Apr 19, 2021 at 05:06:10PM +0800, a397341...@163.com 
> >>>  wrote:
>  From: toseven 
> > [...]
>  +if (ret < 0)
>  +{
>  +fprintf(stderr, "Error occurred in av_packet_add_side_data: 
>  %s\n",
>  +av_err2str(ret));
>  +}
>  +return ret;
> >>> 
> >>> the { } placing style mismatches whats used in FFmpeg (i dont mind but 
> >>> some people do mind)
> >>> 
> >>> more general, how much code coverage is gained with these 2 fuzzers 
> >>> compared to what already exists ?
> >>> 
> >>> thanks
> >> 
> >> Okay, I will modify my style to adopt for FFmpeg. What is more, I didn’t 
> >> compare the code coverage between them. Do I have to do this?  I mainly 
> >> refer to the fate test from libavcodec/tests/avpacket.c and 
> >> libavfilter/tests/formats.c.
> > 
> > If code coverage does not improve, what would be the reason for FFmpeg to
> > include the code ?
> 
> Thank your reply. 
> My fuzzing targets call the new API interfaces, which are not used by the 
> existing fuzzing target. Though I don’t do the related experiment, code 
> coverage should improve. 

Current fuzzer coverage can be seen here:
https://storage.googleapis.com/oss-fuzz-coverage/ffmpeg/reports/20210420/linux/src/ffmpeg/report.html

You are adding 2 targets
target_avpacket_fuzzer:
this calls
av_packet_side_data_name, av_packet_add_side_data, av_packet_free, 
av_packet_clone, av_grow_packet, av_new_packet, av_packet_from_data, 
From these all but av_packet_clone and av_packet_side_data_name are covered 
already
av_packet_side_data_name() is called with a fixed argument in your code

target_formats_fuzzer:
this calls av_get_channel_layout_string, ff_parse_channel_layout
the first is already covered the second is in libavfilter
libavfilter needs to be fuzzed, such fuzzing would involve building
filter chains or networks based on fuzzer input.
A 2nd set of libavfilter fuzzers should similar to libavcodec fuzzers
generate 1 fuzzer generically for each avfilter similarly to how decoders
from libavcodec are fuzzed.
Such libavfilter fuzzers would then also test most functions within libavfilter

More generally about coverage.
If you where in my position what would you want for additional fuzzers ?
maximally increased coverage with mininmal effort ?
I belive this would be achieved with generic fuzzing of filters similar to how
decoders are fuzzed currently. But thats a bit bigger effort 

I see the gsoc page says connecting 2 fate tests to fuzzing can be used as
qualification task. 
For connecting such tests, the fate test and the fuzzer should use shared
code and not duplicate. One way that can work is that the fate test takes
some input and that input is fixed for fate but can change when used for fuzzing
again, the more coverage we can achieve with as little effort the better.
Basically dont be afraid to submit a small amount of code because in fact
i would be more impressed if you can connect fate test(s) with little code
to the fuzzer than with alot of code.

Thanks

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

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin


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 1/3] avformat/mpegtsenc: Fix mpegts_write_pes() for private_stream_2 and other types

2021-04-22 Thread Mao Hata 

On 2021/04/22 12:36, zheng qian wrote:

On Thu, Apr 22, 2021 at 12:11 PM Mao Hata  wrote:


PES_packet_length seems to be inaccurate, because "header_len + 3" has
already been added to the variable "len".


I'm sorry for that and I'll submit v3 patch set later.
Please tell me if there're any other problems.

Regards,
zheng



Thank you, so I'll wait for v3 patch. Except for the "len" issue, I have 
not found any other issues so far.
This patch (and the patches you are submitting in parallel) should solve 
many problems with direct copying of ARIB subtitles!

___
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] Gsoc: add the two fuzzy targets

2021-04-22 Thread Heng Zhang 


> 在 2021年4月20日，下午7:12，Michael Niedermayer  写道：
> 
> On Tue, Apr 20, 2021 at 12:34:13PM +0800, Heng Zhang wrote:
>> 
>> 
>>> 在 2021年4月19日，下午5:47，Michael Niedermayer  写道：
>>> 
>>> On Mon, Apr 19, 2021 at 05:06:10PM +0800, a397341...@163.com 
>>>  wrote:
 From: toseven 
> [...]
 +if (ret < 0)
 +{
 +fprintf(stderr, "Error occurred in av_packet_add_side_data: %s\n",
 +av_err2str(ret));
 +}
 +return ret;
>>> 
>>> the { } placing style mismatches whats used in FFmpeg (i dont mind but some 
>>> people do mind)
>>> 
>>> more general, how much code coverage is gained with these 2 fuzzers 
>>> compared to what already exists ?
>>> 
>>> thanks
>> 
>> Okay, I will modify my style to adopt for FFmpeg. What is more, I didn’t 
>> compare the code coverage between them. Do I have to do this?  I mainly 
>> refer to the fate test from libavcodec/tests/avpacket.c and 
>> libavfilter/tests/formats.c.
> 
> If code coverage does not improve, what would be the reason for FFmpeg to
> include the code ?

Thank your reply. 
My fuzzing targets call the new API interfaces, which are not used by the 
existing fuzzing target. Though I don’t do the related experiment, code 
coverage should improve. 

> 
> Thanks
> 
> [...]
> -- 
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> Republics decline into democracies and democracies degenerate into
> despotisms. -- Aristotle
> ___
> 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".