| Message ID | 20230325191529.10578-1-anton@khirnov.net |
|---|---|
| State | Accepted |
| Commit | 8e23a62eff08715ce5ec021c7b4de7ad9c716089 |
| Headers | show |
| Series | [FFmpeg-devel,01/23] fftools/ffmpeg: drop InputStream.processing_needed | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | fail | Make fate failed |
On 3/25/2023 4:15 PM, Anton Khirnov wrote: > We do not support data encoders, so this should never be reached. nit: The abort() below could be an av_assert0(0) instead. It's the only abort() in the file, whereas av_assert0() is used a lot. > --- > fftools/ffmpeg.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > index f00b2d44e4..a424c69725 100644 > --- a/fftools/ffmpeg.c > +++ b/fftools/ffmpeg.c > @@ -3174,8 +3174,6 @@ static int init_output_stream_encode(OutputStream *ost, AVFrame *frame) > } > } > > - break; > - case AVMEDIA_TYPE_DATA: > break; > default: > abort();
On Sat, Mar 25, 2023 at 08:15:14PM +0100, Anton Khirnov wrote: > The code currently uses lavfi for this, which creates a sort of > configuration dependency loop - the encoder should be ideally > initialized with information from the first audio frame, but to get this > frame one needs to first open the encoder to know the frame size. This > necessitates an awkward workaround, which causes audio handling to be > different from video. > > With this change, audio encoder initialization is congruent with video. > --- > fftools/ffmpeg.c | 58 ++++++++------------------------------- > fftools/ffmpeg_filter.c | 8 ------ > fftools/ffmpeg_mux_init.c | 19 +++++++++---- > 3 files changed, 25 insertions(+), 60 deletions(-) this results in the following to be apparently stuck ffmpeg -y -i https://samples.ffmpeg.org/V-codecs/geov.avi -t 1 file.avi [...]
Quoting James Almer (2023-03-25 20:43:46) > On 3/25/2023 4:15 PM, Anton Khirnov wrote: > > We do not support data encoders, so this should never be reached. > > nit: The abort() below could be an av_assert0(0) instead. It's the only > abort() in the file, whereas av_assert0() is used a lot. I did notice that too and I agree it should be changed, but not in this patch.
Quoting Anton Khirnov (2023-03-25 20:15:27) > Drop unneeded ctype.h and math.h. > Group all system headers together. > Sort unconditional includes alphabetically. > --- > fftools/ffmpeg.c | 65 +++++++++++++++++++++++------------------------- > 1 file changed, 31 insertions(+), 34 deletions(-) As per discussion on IRC with Andreas and James, there seems to be little advantage to splitting this patch and the next one, so unless someone objects I will squash them before pushing.
On Sat, Mar 25, 2023 at 10:43:49PM +0100, Michael Niedermayer wrote: > On Sat, Mar 25, 2023 at 08:15:14PM +0100, Anton Khirnov wrote: > > The code currently uses lavfi for this, which creates a sort of > > configuration dependency loop - the encoder should be ideally > > initialized with information from the first audio frame, but to get this > > frame one needs to first open the encoder to know the frame size. This > > necessitates an awkward workaround, which causes audio handling to be > > different from video. > > > > With this change, audio encoder initialization is congruent with video. > > --- > > fftools/ffmpeg.c | 58 ++++++++------------------------------- > > fftools/ffmpeg_filter.c | 8 ------ > > fftools/ffmpeg_mux_init.c | 19 +++++++++---- > > 3 files changed, 25 insertions(+), 60 deletions(-) > > this results in the following to be apparently stuck > > ffmpeg -y -i https://samples.ffmpeg.org/V-codecs/geov.avi -t 1 file.avi This patch causes more issues it seems the following: ./ffmpeg -i AnivisionLogo.bik -t 1 -y test.avi see https://samples.ffmpeg.org/game-formats/bink/ produces: ==9868== Process terminating with default action of signal 11 (SIGSEGV) ==9868== General Protection Fault ==9868== at 0x11D3CAD: ??? (in ffmpeg/ffmpeg_g) ==9868== by 0x9ACA31: mp3lame_encode_frame (in ffmpeg/ffmpeg_g) ==9868== by 0x882EC4: ff_encode_encode_cb (in ffmpeg/ffmpeg_g) ==9868== by 0x8832DD: encode_receive_packet_internal (in ffmpeg/ffmpeg_g) ==9868== by 0x8834BF: avcodec_send_frame (in ffmpeg/ffmpeg_g) ==9868== by 0x31A281: encode_frame (in ffmpeg/ffmpeg_g) ==9868== by 0x31F901: transcode (in ffmpeg/ffmpeg_g) ==9868== by 0x2F0843: main (in ffmpeg/ffmpeg_g) [...]
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c index 438bee8fef..aa9284ecd5 100644 --- a/fftools/ffmpeg.c +++ b/fftools/ffmpeg.c @@ -3648,7 +3648,7 @@ static void decode_flush(InputFile *ifile) InputStream *ist = ifile->streams[i]; int ret; - if (!ist->processing_needed) + if (ist->discard) continue; do { @@ -3793,7 +3793,7 @@ static int process_input(int file_index) for (i = 0; i < ifile->nb_streams; i++) { ist = ifile->streams[i]; - if (ist->processing_needed) { + if (!ist->discard) { ret = process_input_packet(ist, NULL, 0); if (ret>0) return 0; diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h index 823218e214..791deedc07 100644 --- a/fftools/ffmpeg.h +++ b/fftools/ffmpeg.h @@ -342,7 +342,6 @@ typedef struct InputStream { int decoding_needed; /* non zero if the packets must be decoded in 'raw_fifo', see DECODING_FOR_* */ #define DECODING_FOR_OST 1 #define DECODING_FOR_FILTER 2 - int processing_needed; /* non zero if the packets must be processed */ // should attach FrameData as opaque_ref after decoding int want_frame_data; diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c index 4fde120067..314b89b585 100644 --- a/fftools/ffmpeg_filter.c +++ b/fftools/ffmpeg_filter.c @@ -296,7 +296,6 @@ static void init_input_filter(FilterGraph *fg, AVFilterInOut *in) ist->discard = 0; ist->decoding_needed |= DECODING_FOR_FILTER; - ist->processing_needed = 1; ist->st->discard = AVDISCARD_NONE; ifilter = ALLOC_ARRAY_ELEM(fg->inputs, fg->nb_inputs); diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c index 09d24ba8e5..ebc17059f9 100644 --- a/fftools/ffmpeg_mux_init.c +++ b/fftools/ffmpeg_mux_init.c @@ -2283,7 +2283,6 @@ int of_open(const OptionsContext *o, const char *filename) if (ost->enc_ctx && ost->ist) { InputStream *ist = ost->ist; ist->decoding_needed |= DECODING_FOR_OST; - ist->processing_needed = 1; if (ost->st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO || ost->st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) { @@ -2294,8 +2293,6 @@ int of_open(const OptionsContext *o, const char *filename) exit_program(1); } } - } else if (ost->ist) { - ost->ist->processing_needed = 1; } /* set the filter output constraints */