diff mbox series

[FFmpeg-devel,8/9] ffmpeg: drop a useless assignment

Message ID 20211115152400.5522-8-anton@khirnov.net
State New
Headers show
Series [FFmpeg-devel,1/9] ffmpeg: do not copy chroma_sample_location from the input stream | expand

Checks

Context Check Description
andriy/configurex86 warning Failed to apply patch
andriy/configureppc warning Failed to apply patch

Commit Message

Anton Khirnov Nov. 15, 2021, 3:23 p.m. UTC 
bits_per_raw_sample is already set in new_video_stream(), so this code
has no effect.
---
 fftools/ffmpeg.c     | 7 -------
 fftools/ffmpeg.h     | 1 -
 fftools/ffmpeg_opt.c | 2 +-
 3 files changed, 1 insertion(+), 9 deletions(-)

Comments

Michael Niedermayer Nov. 16, 2021, 1:46 p.m. UTC | #1 
On Mon, Nov 15, 2021 at 04:23:59PM +0100, Anton Khirnov wrote:
> bits_per_raw_sample is already set in new_video_stream(), so this code
> has no effect.
> ---
>  fftools/ffmpeg.c     | 7 -------
>  fftools/ffmpeg.h     | 1 -
>  fftools/ffmpeg_opt.c | 2 +-
>  3 files changed, 1 insertion(+), 9 deletions(-)

maybe i messed up or missing something but this breaks fate for me

TEST    lavf-pam
TEST    lavf-rgba.pam
TEST    lavf-gray.pam
TEST    lavf-gray16be.pam
TEST    lavf-rgb48be.pam
--- ./tests/ref/lavf/rgb48le.dpx	2021-11-01 13:29:46.883005610 +0100
+++ tests/data/fate/lavf-rgb48le.dpx	2021-11-16 14:34:59.063907946 +0100
@@ -1,3 +1,2 @@
-075963c3c08978b6a20555ba09161434 *tests/data/images/rgb48le.dpx/02.rgb48le.dpx
-tests/data/images/rgb48le.dpx/%02d.rgb48le.dpx CRC=0xe5b9c023
-609920 tests/data/images/rgb48le.dpx/02.rgb48le.dpx
+tests/data/images/rgb48le.dpx/%02d.rgb48le.dpx 
+
Test lavf-rgb48le.dpx failed. Look at tests/data/fate/lavf-rgb48le.dpx.err for details.
tests/Makefile:257: recipe for target 'fate-lavf-rgb48le.dpx' failed
make: *** [fate-lavf-rgb48le.dpx] Error 1
make: *** Waiting for unfinished jobs....
--- ./tests/ref/lavf/rgb48le_10.dpx	2021-11-01 13:29:46.883005610 +0100
+++ tests/data/fate/lavf-rgb48le_10.dpx	2021-11-16 14:34:59.075908062 +0100
@@ -1,3 +1,2 @@
-b9f22728f8ff393bf30cf6cbd624fa95 *tests/data/images/rgb48le_10.dpx/02.rgb48le_10.dpx
-tests/data/images/rgb48le_10.dpx/%02d.rgb48le_10.dpx CRC=0xf38d5830
-407168 tests/data/images/rgb48le_10.dpx/02.rgb48le_10.dpx
+tests/data/images/rgb48le_10.dpx/%02d.rgb48le_10.dpx 
+
Test lavf-rgb48le_10.dpx failed. Look at tests/data/fate/lavf-rgb48le_10.dpx.err for details.
tests/Makefile:257: recipe for target 'fate-lavf-rgb48le_10.dpx' failed
make: *** [fate-lavf-rgb48le_10.dpx] Error 1
Anton Khirnov Nov. 16, 2021, 7:11 p.m. UTC | #2 
Quoting Michael Niedermayer (2021-11-16 14:46:43)
> On Mon, Nov 15, 2021 at 04:23:59PM +0100, Anton Khirnov wrote:
> > bits_per_raw_sample is already set in new_video_stream(), so this code
> > has no effect.
> > ---
> >  fftools/ffmpeg.c     | 7 -------
> >  fftools/ffmpeg.h     | 1 -
> >  fftools/ffmpeg_opt.c | 2 +-
> >  3 files changed, 1 insertion(+), 9 deletions(-)
> 
> maybe i messed up or missing something but this breaks fate for me

Cannot reproduce here, FATE passes for me.
Any ideas what could be the cause?
Michael Niedermayer Nov. 16, 2021, 10:26 p.m. UTC | #3 
On Tue, Nov 16, 2021 at 08:11:41PM +0100, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2021-11-16 14:46:43)
> > On Mon, Nov 15, 2021 at 04:23:59PM +0100, Anton Khirnov wrote:
> > > bits_per_raw_sample is already set in new_video_stream(), so this code
> > > has no effect.
> > > ---
> > >  fftools/ffmpeg.c     | 7 -------
> > >  fftools/ffmpeg.h     | 1 -
> > >  fftools/ffmpeg_opt.c | 2 +-
> > >  3 files changed, 1 insertion(+), 9 deletions(-)
> > 
> > maybe i messed up or missing something but this breaks fate for me
> 
> Cannot reproduce here, FATE passes for me.
> Any ideas what could be the cause?

id have to re-look but i think it only failed after a 
make distclean
if it doesnt reproduce with make distclean then ill take a deeper
look (tomorrow or so)

thx

[...]
Michael Niedermayer Nov. 17, 2021, 2:27 p.m. UTC | #4 
On Tue, Nov 16, 2021 at 11:26:26PM +0100, Michael Niedermayer wrote:
> On Tue, Nov 16, 2021 at 08:11:41PM +0100, Anton Khirnov wrote:
> > Quoting Michael Niedermayer (2021-11-16 14:46:43)
> > > On Mon, Nov 15, 2021 at 04:23:59PM +0100, Anton Khirnov wrote:
> > > > bits_per_raw_sample is already set in new_video_stream(), so this code
> > > > has no effect.
> > > > ---
> > > >  fftools/ffmpeg.c     | 7 -------
> > > >  fftools/ffmpeg.h     | 1 -
> > > >  fftools/ffmpeg_opt.c | 2 +-
> > > >  3 files changed, 1 insertion(+), 9 deletions(-)
> > > 
> > > maybe i messed up or missing something but this breaks fate for me
> > 
> > Cannot reproduce here, FATE passes for me.
> > Any ideas what could be the cause?
> 
> id have to re-look but i think it only failed after a 
> make distclean
> if it doesnt reproduce with make distclean then ill take a deeper
> look (tomorrow or so)

ive just taken another quick look and simplified this down to whats causing it
heres a better testcase

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 7f9f98222f2..dc86c9859a2 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -3453,9 +3453,10 @@ static int init_output_stream_encode(OutputStream *ost, AVFrame *frame)
             enc_ctx->width   != dec_ctx->width  ||
             enc_ctx->height  != dec_ctx->height ||
             enc_ctx->pix_fmt != dec_ctx->pix_fmt) {
-            enc_ctx->bits_per_raw_sample = frame_bits_per_raw_sample;
+//             enc_ctx->bits_per_raw_sample = frame_bits_per_raw_sample;
         }
 
         // Field order: autodetection
         if (frame) {
             if (enc_ctx->flags & (AV_CODEC_FLAG_INTERLACED_DCT | AV_CODEC_FLAG_INTERLACED_ME) &&

with this:

ffmpeg  -y  -f image2 -c:v pgmyuv -i /tests/vsynth1/%02d.pgm -pix_fmt rgb48le /tmp/%02d.rgb48le.dpx
 
 
Video encoding failed
Conversion failed!

(thats with the other patches in this set before this one applied) but no
other patches applied
diff mbox series

Patch

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 0f2fe192ac..c0ab78608a 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -3451,13 +3451,6 @@  static int init_output_stream_encode(OutputStream *ost, AVFrame *frame)
 
         ost->st->avg_frame_rate = ost->frame_rate;
 
-        if (!dec_ctx ||
-            enc_ctx->width   != dec_ctx->width  ||
-            enc_ctx->height  != dec_ctx->height ||
-            enc_ctx->pix_fmt != dec_ctx->pix_fmt) {
-            enc_ctx->bits_per_raw_sample = frame_bits_per_raw_sample;
-        }
-
         // Field order: autodetection
         if (frame) {
             if (enc_ctx->flags & (AV_CODEC_FLAG_INTERLACED_DCT | AV_CODEC_FLAG_INTERLACED_ME) &&
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 30225e9ffe..2da7b1b7df 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -625,7 +625,6 @@  extern int print_stats;
 extern int64_t stats_period;
 extern int qp_hist;
 extern int stdin_interaction;
-extern int frame_bits_per_raw_sample;
 extern AVIOContext *progress_avio;
 extern float max_error_rate;
 extern char *videotoolbox_pixfmt;
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 54f6279bf5..c16f5e8672 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -166,7 +166,7 @@  int abort_on_flags    = 0;
 int print_stats       = -1;
 int qp_hist           = 0;
 int stdin_interaction = 1;
-int frame_bits_per_raw_sample = 0;
+static int frame_bits_per_raw_sample = 0;
 float max_error_rate  = 2.0/3;
 char *filter_nbthreads;
 int filter_complex_nbthreads = 0;