[FFmpeg-devel] [PATCH 07/25] fftools/ffmpeg: remove OutputStream.stream_copy

[Andreas Rheinhardt]

Anton Khirnov:
> Quoting Andreas Rheinhardt (2022-08-04 14:54:41)
>> Michael Niedermayer:
>>> On Wed, Aug 03, 2022 at 03:58:26PM +0200, Anton Khirnov wrote:
>>>> There are currently three possible modes for an output stream:
>>>> 1) The stream is produced by encoding output from some filtergraph. This
>>>>    is true when ost->enc_ctx != NULL, or equivalently when
>>>>    ost->encoding_needed != 0.
>>>> 2) The stream is produced by copying some input stream's packets. This
>>>>    is true when ost->enc_ctx == NULL && ost->source_index >= 0.
>>>> 3) The stream is produced by attaching some file directly. This is true
>>>>    when ost->enc_ctx == NULL && ost->source_index < 0.
>>>>
>>>> OutputStream.stream_copy is currently used to identify case 2), and
>>>> sometimes to confusingly (or even incorrectly) identify case 1). Remove
>>>> it, replacing its usage with checking enc_ctx/source_index values.
>>>> ---
>>>>  fftools/ffmpeg.c     | 23 +++++++++--------------
>>>>  fftools/ffmpeg.h     |  1 -
>>>>  fftools/ffmpeg_opt.c | 33 ++++++++++++---------------------
>>>>  3 files changed, 21 insertions(+), 36 deletions(-)
>>>
>>> seems to break build with shared libs:
>>>
>>> CC	fftools/ffmpeg.o
>>> src/fftools/ffmpeg.c:405:32: warning: suggest braces around initialization of subobject [-Wmissing-braces]
>>>     struct sigaction action = {0};
>>>                                ^
>>>                                {}
>>> src/fftools/ffmpeg.c:1612:37: error: address argument to atomic operation must be a pointer to non-const _Atomic type ('const atomic_uint_least64_t *' (aka 'const _Atomic(uint_least64_t) *') invalid)
>>>             uint64_t frame_number = atomic_load(&ost->packets_written);
>>>                                     ^           ~~~~~~~~~~~~~~~~~~~~~
>>> /usr/lib/llvm-6.0/lib/clang/6.0.0/include/stdatomic.h:134:29: note: expanded from macro 'atomic_load'
>>> #define atomic_load(object) __c11_atomic_load(object, __ATOMIC_SEQ_CST)
>>>                             ^                 ~~~~~~
>>> 1 warning and 1 error generated.
>>> src/ffbuild/common.mak:81: recipe for target 'fftools/ffmpeg.o' failed
>>> make: *** [fftools/ffmpeg.o] Error 1
>>>
>>> [...]
>>>
>>
>> This has nothing to do with shared libs. It is just that C11 defines
>> atomic_load in such a way that it does not allow a pointer to const
>> atomic; presumably because atomic types might be emulated via mutexes in
>> which case even a read involves a write.
> 
> I don't even get a warning with gcc 11 or clang 13.
> 

Yes, same for me (this is meant literally:
https://ffmpeg.org/pipermail/ffmpeg-devel/2022-July/298360.html). Maybe
the spec has been modified since then:
https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1807.htm and
https://en.cppreference.com/w/c/atomic/atomic_load indicate this.

- Andreas