diff mbox series

[FFmpeg-devel] fftools: Avoid build failure on OSX 10.6 with gcc 4.2.1

Message ID 20230715183336.8052-1-pkoshevoy@gmail.com
State New
Headers show
Series [FFmpeg-devel] fftools: Avoid build failure on OSX 10.6 with gcc 4.2.1 | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Pavel Koshevoy July 15, 2023, 6:33 p.m. UTC 
... and make code more readable / human friendly.
---
 fftools/ffmpeg_mux_init.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Pavel Koshevoy July 18, 2023, 2:49 p.m. UTC | #1 
ping ... could someone review this please

On Sat, Jul 15, 2023 at 12:33 PM Pavel Koshevoy <pkoshevoy@gmail.com> wrote:

> ... and make code more readable / human friendly.
> ---
>  fftools/ffmpeg_mux_init.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
> index 6458414b5f..bade35896f 100644
> --- a/fftools/ffmpeg_mux_init.c
> +++ b/fftools/ffmpeg_mux_init.c
> @@ -1648,7 +1648,8 @@ read_fail:
>
>  static int create_streams(Muxer *mux, const OptionsContext *o)
>  {
> -    static const int (*map_func[])(Muxer *mux, const OptionsContext *o) =
> {
> +    typedef int(*TMapFunc)(Muxer *, const OptionsContext *);
> +    static const TMapFunc map_func[] = {
>          [AVMEDIA_TYPE_VIDEO]    = map_auto_video,
>          [AVMEDIA_TYPE_AUDIO]    = map_auto_audio,
>          [AVMEDIA_TYPE_SUBTITLE] = map_auto_subtitle,
> --
> 2.35.3
>
>
Andreas Rheinhardt July 18, 2023, 10 p.m. UTC | #2 
Pavel Koshevoy:
> ping ... could someone review this please
> 
> On Sat, Jul 15, 2023 at 12:33 PM Pavel Koshevoy <pkoshevoy@gmail.com> wrote:
> 
>> ... and make code more readable / human friendly.
>> ---
>>  fftools/ffmpeg_mux_init.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
>> index 6458414b5f..bade35896f 100644
>> --- a/fftools/ffmpeg_mux_init.c
>> +++ b/fftools/ffmpeg_mux_init.c
>> @@ -1648,7 +1648,8 @@ read_fail:
>>
>>  static int create_streams(Muxer *mux, const OptionsContext *o)
>>  {
>> -    static const int (*map_func[])(Muxer *mux, const OptionsContext *o) =
>> {
>> +    typedef int(*TMapFunc)(Muxer *, const OptionsContext *);
>> +    static const TMapFunc map_func[] = {
>>          [AVMEDIA_TYPE_VIDEO]    = map_auto_video,
>>          [AVMEDIA_TYPE_AUDIO]    = map_auto_audio,
>>          [AVMEDIA_TYPE_SUBTITLE] = map_auto_subtitle,
>> --
>> 2.35.3
>>
>>

This issue has already been fixed in
153cf85b246a7931fac6344f2189cd268ca9e0aa.

- Andreas
Pavel Koshevoy July 18, 2023, 11:46 p.m. UTC | #3 
On Tue, Jul 18, 2023 at 3:59 PM Andreas Rheinhardt <
andreas.rheinhardt@outlook.com> wrote:

> Pavel Koshevoy:
> > ping ... could someone review this please
> >
> > On Sat, Jul 15, 2023 at 12:33 PM Pavel Koshevoy <pkoshevoy@gmail.com>
> wrote:
> >
> >> ... and make code more readable / human friendly.
> >> ---
> >>  fftools/ffmpeg_mux_init.c | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
> >> index 6458414b5f..bade35896f 100644
> >> --- a/fftools/ffmpeg_mux_init.c
> >> +++ b/fftools/ffmpeg_mux_init.c
> >> @@ -1648,7 +1648,8 @@ read_fail:
> >>
> >>  static int create_streams(Muxer *mux, const OptionsContext *o)
> >>  {
> >> -    static const int (*map_func[])(Muxer *mux, const OptionsContext
> *o) =
> >> {
> >> +    typedef int(*TMapFunc)(Muxer *, const OptionsContext *);
> >> +    static const TMapFunc map_func[] = {
> >>          [AVMEDIA_TYPE_VIDEO]    = map_auto_video,
> >>          [AVMEDIA_TYPE_AUDIO]    = map_auto_audio,
> >>          [AVMEDIA_TYPE_SUBTITLE] = map_auto_subtitle,
> >> --
> >> 2.35.3
> >>
> >>
>
> This issue has already been fixed in
> 153cf85b246a7931fac6344f2189cd268ca9e0aa.
>

I see ... I can check if it compiles on OSX 10.6 over the weekend.
I've got to say that a typedef for the function pointer is arguably a
better solution, as it doesn't require nearly as much mental gymnastics to
parse what is happening on that line.
Code should be easy to read, and C function pointer syntax is very awkward
... combining it all into an array declaration at the same time is just
making it worse.

... but this is not my code to maintain, so feel free to ignore.

Pavel.
diff mbox series

Patch

diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index 6458414b5f..bade35896f 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -1648,7 +1648,8 @@  read_fail:
 
 static int create_streams(Muxer *mux, const OptionsContext *o)
 {
-    static const int (*map_func[])(Muxer *mux, const OptionsContext *o) = {
+    typedef int(*TMapFunc)(Muxer *, const OptionsContext *);
+    static const TMapFunc map_func[] = {
         [AVMEDIA_TYPE_VIDEO]    = map_auto_video,
         [AVMEDIA_TYPE_AUDIO]    = map_auto_audio,
         [AVMEDIA_TYPE_SUBTITLE] = map_auto_subtitle,