mod_av: avformat.c lines 458-459 reference private field within AVOutputFormat

[Korynkai]

The following code is apparently referencing a private field within the AVOutputFormat structure of libavformat, which is described as "[not] part of the public API", "may not be used outside of libavformat" and "can be changed and removed at will":

freeswitch/src/mod/applications/mod_av/avformat.c

Line 458 in b74245d

if (s->oformat->priv_data_size > 0) {

freeswitch/src/mod/applications/mod_av/avformat.c

Line 459 in b74245d

s->priv_data = av_mallocz(s->oformat->priv_data_size);

As described here:
https://github.com/FFmpeg/FFmpeg/blob/566aa38d98f5f492995127e82ab9a516f59bf952/libavformat/avformat.h#L540

typedef struct AVOutputFormat {

...continued...

    /*****************************************************************
     * No fields below this line are part of the public API. They
     * may not be used outside of libavformat and can be changed and
     * removed at will.
     * New public fields should be added right above.
     *****************************************************************
     */
    /**
     * size of private data so that it can be allocated in the wrapper
     */
    int priv_data_size;

...continued...

} AVOutputFormat;

mod_av, as-is, causes the following error when building against FFmpeg 6.0, as this field has apparently been removed from FFmpeg 6.0:

avformat.c: In function 'mod_avformat_alloc_output_context2':
avformat.c:458:23: error: 'const struct AVOutputFormat' has no member named 'priv_data_size'
  458 |         if (s->oformat->priv_data_size > 0) {
      |                       ^~
avformat.c:459:53: error: 'const struct AVOutputFormat' has no member named 'priv_data_size'
  459 |                 s->priv_data = av_mallocz(s->oformat->priv_data_size);
      |                                                     ^~
make[4]: *** [Makefile:1026: libavmod_la-avformat.lo] Error 1

With the following patch, everything seems to build correctly:

diff -Naur freeswitch-1.10.10.pristine/src/mod/applications/mod_av/avformat.c freeswitch-1.10.10/src/mod/applications/mod_av/avformat.c
--- freeswitch-1.10.10.pristine/src/mod/applications/mod_av/avformat.c  2023-08-14 17:45:02.529331160 +0000
+++ freeswitch-1.10.10/src/mod/applications/mod_av/avformat.c   2023-08-14 23:52:55.012169884 +0000
@@ -455,8 +455,10 @@
        }
 
        s->oformat = oformat;
+#if (LIBAVFORMAT_VERSION_MAJOR < LIBAVFORMAT_N)
        if (s->oformat->priv_data_size > 0) {
                s->priv_data = av_mallocz(s->oformat->priv_data_size);
+
                if (!s->priv_data) {
                        goto nomem;
                }
@@ -468,6 +470,7 @@
        } else {
                s->priv_data = NULL;
        }
+#endif
 
        if (filename) {
 #if (LIBAVCODEC_VERSION_INT < AV_VERSION_INT(58,7,100))
diff -Naur freeswitch-1.10.10.pristine/src/mod/applications/mod_av/mod_av.h freeswitch-1.10.10/src/mod/applications/mod_av/mod_av.h
--- freeswitch-1.10.10.pristine/src/mod/applications/mod_av/mod_av.h    2023-08-14 17:45:02.529331160 +0000
+++ freeswitch-1.10.10/src/mod/applications/mod_av/mod_av.h     2023-08-14 19:40:21.271938704 +0000
@@ -42,6 +42,7 @@
 
 #define LIBAVCODEC_V 59
 #define LIBAVFORMAT_V 59
+#define LIBAVFORMAT_N 60
 #define LIBAVUTIL_V 57
 
 struct mod_av_globals {

What is the purpose of referencing the priv_data_size field within the AVOutputFormat structure, and can this be averted? Can this specific conditional block be safely removed by package maintainers such as myself, as in the above patch, or will it cause some form of runtime issues or undefined behavior? I have attempted to examine this code and have not quite determined the purpose it serves.. It seems as if it is allocating the number of bytes its value indicates to the priv_data field of the allocated AVFormatContext object and then the priv_data pointer is assigned to the priv_class object within the AVOutputFormat structure, then av_opt_set_defaults() is called on the priv_data pointer if the priv_class object exists...

Can someone help me determine the purpose of this and assist in developing a patch and/or pull request that would fix this?

[fx02]

We did video testing/recording and had no issues with this patch for now.

[tynli]

With FreeSWITCH v1.10.11, this patch does not solve the whole problem, compiling mod_av also throws the following errors (because warnings are treated as errors while compiling FreeSWITCH):

making all mod_av
make[4]: Entering directory '/usr/local/src/freeswitch/src/mod/applications/mod_av'
  CC       libavmod_la-mod_av.lo
  CC       libavmod_la-avformat.lo
avformat.c: In function 'add_stream':
avformat.c:626:25: error: 'ticks_per_frame' is deprecated [-Werror=deprecated-declarations]
  626 |                         c->ticks_per_frame = 2;
      |                         ^
In file included from avformat.c:37:
/usr/include/x86_64-linux-gnu/libavcodec/avcodec.h:579:9: note: declared here
  579 |     int ticks_per_frame;
      |         ^~~~~~~~~~~~~~~
avformat.c: In function 'av_file_read_video':
avformat.c:3208:17: error: 'ticks_per_frame' is deprecated [-Werror=deprecated-declarations]
 3208 |                 ticks = cp ? cp->repeat_pict + 1 : c->ticks_per_frame;
      |                 ^~~~~
/usr/include/x86_64-linux-gnu/libavcodec/avcodec.h:579:9: note: declared here
  579 |     int ticks_per_frame;
      |         ^~~~~~~~~~~~~~~
avformat.c:3214:25: error: 'ticks_per_frame' is deprecated [-Werror=deprecated-declarations]
 3214 |                         context->video_start_time, ticks, c ? c->ticks_per_frame : -1, st->time_base.num, st->time_base.den, c ? c->time_base.num : -1, c ? c->time_base.den : -1,
      |                         ^~~~~~~
/usr/include/x86_64-linux-gnu/libavcodec/avcodec.h:579:9: note: declared here
  579 |     int ticks_per_frame;
      |         ^~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

[fvthakor]

yes I got same error
while installing freeswitch

make[4]: Entering directory '/usr/src/freeswitch/src/mod/applications/mod_av'
  CC       test/test_mod_av-test_mod_av.o
  CC       libavmod_la-mod_av.lo
  CC       libavmod_la-avformat.lo
avformat.c: In function 'mod_avformat_alloc_output_context2':
avformat.c:458:23: error: 'const struct AVOutputFormat' has no member named 'priv_data_size'
  458 |         if (s->oformat->priv_data_size > 0) {
      |                       ^~
avformat.c:459:53: error: 'const struct AVOutputFormat' has no member named 'priv_data_size'
  459 |                 s->priv_data = av_mallocz(s->oformat->priv_data_size);
      |                                                     ^~
avformat.c: In function 'add_stream':
avformat.c:624:25: error: 'ticks_per_frame' is deprecated [-Werror=deprecated-declarations]
  624 |                         c->ticks_per_frame = 2;
      |                         ^
In file included from avformat.c:37:
/usr/include/x86_64-linux-gnu/libavcodec/avcodec.h:579:9: note: declared here
  579 |     int ticks_per_frame;
      |         ^~~~~~~~~~~~~~~
avformat.c: In function 'av_file_read_video':
avformat.c:3206:17: error: 'ticks_per_frame' is deprecated [-Werror=deprecated-declarations]
 3206 |                 ticks = cp ? cp->repeat_pict + 1 : c->ticks_per_frame;
      |                 ^~~~~
/usr/include/x86_64-linux-gnu/libavcodec/avcodec.h:579:9: note: declared here
  579 |     int ticks_per_frame;
      |         ^~~~~~~~~~~~~~~
avformat.c:3212:25: error: 'ticks_per_frame' is deprecated [-Werror=deprecated-declarations]
 3212 |                         context->video_start_time, ticks, c ? c->ticks_per_frame : -1, st->time_base.num, st->time_base.den, c ? c->time_base.num : -1, c ? c->time_base.den : -1,
      |                         ^~~~~~~
/usr/include/x86_64-linux-gnu/libavcodec/avcodec.h:579:9: note: declared here
  579 |     int ticks_per_frame;
      |         ^~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

[codemonkey76]

how can i get around this error? would I configure to compile against an earlier version of ffmpeg? how would i do that?

[Len-PGH]

At the moment, you would need an earlier version of ffmpeg

[Korynkai]

So I seem to have fixed errors and deprecation warnings when compiling against ffmpeg7, however... I am not certain what the appropriate calculation would be for AVCodecContext::ticks_per_frame, nor do I know what the equivalent to AVInputFormat::read_seek or AVInputFormat::read_seek2 would be... It seems the equivalent values for ticks_per_frame may be automatically calculated, but I cannot seem to find that answer... It seems read_seek and read_seek2 is only used for logging, however, so I presume it should be alright to remove it by preprocessor directive...

diff -Naur freeswitch-1.10.12-pristine/src/mod/applications/mod_av/avcodec.c freeswitch-1.10.12/src/mod/applications/mod_av/avcodec.c
--- freeswitch-1.10.12-pristine/src/mod/applications/mod_av/avcodec.c   2024-11-06 07:00:42.737612688 +0000
+++ freeswitch-1.10.12/src/mod/applications/mod_av/avcodec.c    2024-11-06 07:02:45.809730676 +0000
@@ -1226,9 +1226,13 @@
        }
 
        if (context->encoder_ctx) {
+#if (LIBAVCODEC_VERSION_MAJOR < LIBAVCODEC_N)
                if (avcodec_is_open(context->encoder_ctx)) {
                        avcodec_close(context->encoder_ctx);
                }
+#else
+               avcodec_free_context(&(context->encoder_ctx));
+#endif
                av_free(context->encoder_ctx);
                context->encoder_ctx = NULL;
        }
@@ -1319,9 +1323,13 @@
                }
 
                if (context->encoder_ctx) {
+#if (LIBAVCODEC_VERSION_MAJOR < LIBAVCODEC_N)
                        if (avcodec_is_open(context->encoder_ctx)) {
                                avcodec_close(context->encoder_ctx);
                        }
+#else
+                       avcodec_free_context(&(context->encoder_ctx));
+#endif
                        av_free(context->encoder_ctx);
                        context->encoder_ctx = NULL;
                }
@@ -1557,7 +1565,11 @@
                }
 
                 avframe->pict_type = AV_PICTURE_TYPE_I;
+#if (LIBAVUTIL_VERSION_MAJOR < LIBAVUTIL_N)
                 avframe->key_frame = 1;
+#else
+                avframe->flags |= AV_FRAME_FLAG_KEY;
+#endif
                 context->last_keyframe_request = switch_time_now();
        }
 
@@ -1600,9 +1612,14 @@
                }
 #endif
 
+#if (LIBAVUTIL_VERSION_MAJOR < LIBAVUTIL_N)
        if (context->need_key_frame && avframe->key_frame == 1) {
-               avframe->pict_type = 0;
                avframe->key_frame = 0;
+#else
+       if (context->need_key_frame && avframe->flags & AV_FRAME_FLAG_KEY) {
+               avframe->flags ^= AV_FRAME_FLAG_KEY;
+#endif
+               avframe->pict_type = 0;
                context->need_key_frame = 0;
        }
 
@@ -1862,14 +1879,22 @@
 
        switch_buffer_destroy(&context->nalu_buffer);
        if (context->decoder_ctx) {
+#if (LIBAVCODEC_VERSION_MAJOR < LIBAVCODEC_N)
                if (avcodec_is_open(context->decoder_ctx)) avcodec_close(context->decoder_ctx);
+#else
+               avcodec_free_context(&(context->decoder_ctx));
+#endif
                av_free(context->decoder_ctx);
        }
 
        switch_img_free(&context->img);
 
        if (context->encoder_ctx) {
+#if (LIBAVCODEC_VERSION_MAJOR < LIBAVCODEC_N)
                if (avcodec_is_open(context->encoder_ctx)) avcodec_close(context->encoder_ctx);
+#else
+               avcodec_free_context(&(context->encoder_ctx));
+#endif
                av_free(context->encoder_ctx);
        }
 
diff -Naur freeswitch-1.10.12-pristine/src/mod/applications/mod_av/avformat.c freeswitch-1.10.12/src/mod/applications/mod_av/avformat.c
--- freeswitch-1.10.12-pristine/src/mod/applications/mod_av/avformat.c  2024-11-06 07:00:42.737612688 +0000
+++ freeswitch-1.10.12/src/mod/applications/mod_av/avformat.c   2024-11-06 07:07:14.001020789 +0000
@@ -184,6 +184,16 @@
 
 typedef struct av_file_context av_file_context_t;
 
+#if (LIBAVFORMAT_VERSION_MAJOR >= LIBAVFORMAT_N)
+typedef struct FFOutputFormat {
+       int priv_data_size;
+} FFOutputFormat;
+
+static inline int priv_data_size(const AVOutputFormat *fmt)
+{
+    return ((const struct FFOutputFormat*)fmt)->priv_data_size;
+}
+#endif
 
 /**
  * Fill the provided buffer with a string containing a timestamp
@@ -455,8 +465,13 @@
        }
 
        s->oformat = oformat;
+#if (LIBAVFORMAT_VERSION_MAJOR < LIBAVFORMAT_N)
        if (s->oformat->priv_data_size > 0) {
                s->priv_data = av_mallocz(s->oformat->priv_data_size);
+#else
+       if (priv_data_size(s->oformat) > 0) {
+               s->priv_data = av_mallocz(priv_data_size(s->oformat));
+#endif
                if (!s->priv_data) {
                        goto nomem;
                }
@@ -621,7 +636,9 @@
                c->rc_initial_buffer_occupancy = buffer_bytes * 8;
 
                if (codec_id == AV_CODEC_ID_H264) {
+#if (LIBAVCODEC_VERSION_MAJOR < LIBAVCODEC_N)
                        c->ticks_per_frame = 2;
+#endif
 
 
                        c->flags|=AV_CODEC_FLAG_LOOP_FILTER;   // flags=+loop
@@ -1410,8 +1427,10 @@
                switch_goto_status(SWITCH_STATUS_FALSE, err);
        }
 
+#if (LIBAVFORMAT_VERSION_MAJOR < LIBAVFORMAT_N)
        handle->seekable = context->fc->iformat->read_seek2 ? 1 : (context->fc->iformat->read_seek ? 1 : 0);
        switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_INFO, "file %s is %sseekable\n", filename, handle->seekable ? "" : "not ");
+#endif
 
        /** Get information on the input file (number of streams etc.). */
        if ((error = avformat_find_stream_info(context->fc, opts ? &opts : NULL)) < 0) {
@@ -1502,7 +1521,11 @@
 
                switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_ERROR, "Could not open input audio codec channel 2 (error '%s')\n", get_error_text(error, ebuf, sizeof(ebuf)));
                if ((cc = av_get_codec_context(&context->audio_st[0]))) {
+#if (LIBAVCODEC_VERSION_MAJOR < LIBAVCODEC_N)
                        avcodec_close(cc);
+#else
+                       avcodec_free_context(&cc);
+#endif
                }
 
                context->has_audio = 0;
@@ -3084,14 +3107,11 @@
        void *pop;
        MediaStream *mst = &context->video_st;
        AVStream *st = mst->st;
-       int ticks = 0;
        int64_t max_delta = 1 * AV_TIME_BASE; // 1 second
        switch_status_t status = SWITCH_STATUS_SUCCESS;
        double fl_to = 0.02;
        int do_fl = 0;
        int smaller_ts = context->read_fps;
-       AVCodecContext *c = NULL;
-       AVCodecParserContext *cp = NULL;
 
        if (!context->has_video) return SWITCH_STATUS_FALSE;
 
@@ -3199,6 +3219,10 @@
        }
 #endif
 
+#if (LIBAVFORMAT_VERSION_MAJOR < LIBAVFORMAT_N)
+       int ticks = 0;
+       AVCodecContext *c = NULL;
+       AVCodecParserContext *cp = NULL;
        if ((c = av_get_codec_context(mst)) && c->time_base.num) {
                cp = av_stream_get_parser(st);
                ticks = cp ? cp->repeat_pict + 1 : c->ticks_per_frame;
@@ -3210,6 +3234,7 @@
                        context->video_start_time, ticks, c ? c->ticks_per_frame : -1, st->time_base.num, st->time_base.den, c ? c->time_base.num : -1, c ? c->time_base.den : -1,
                        st->start_time, st->duration == AV_NOPTS_VALUE ? context->fc->duration / AV_TIME_BASE * 1000 : st->duration, st->nb_frames, av_q2d(st->time_base));
        }
+#endif
 
  again:
 
diff -Naur freeswitch-1.10.12-pristine/src/mod/applications/mod_av/mod_av.h freeswitch-1.10.12/src/mod/applications/mod_av/mod_av.h
--- freeswitch-1.10.12-pristine/src/mod/applications/mod_av/mod_av.h    2024-11-06 07:00:42.737612688 +0000
+++ freeswitch-1.10.12/src/mod/applications/mod_av/mod_av.h     2024-11-06 07:02:45.809730676 +0000
@@ -42,7 +42,10 @@
 
 #define LIBAVCODEC_V 59
 #define LIBAVFORMAT_V 59
+#define LIBAVCODEC_N 60
+#define LIBAVFORMAT_N 60
 #define LIBAVUTIL_V 57
+#define LIBAVUTIL_N 59
 
 struct mod_av_globals {
        int debug;

[Korynkai]

@FlyingRain what version of ffmpeg are you trying to build against? That patch was made for ffmpeg 7.0, some of the version detection might be a little inaccurate compared to when things were deprecated.

[FlyingRain]

@FlyingRain what version of ffmpeg are you trying to build against? That patch was made for ffmpeg 7.0, some of the version detection might be a little inaccurate compared to when things were deprecated.

yeh i ignored the version of ffmpeg.question had been resolved,i modify the deprecated param depend on your patch.Thanks !

[Korynkai]

@FlyingRain you never mentioned which version of ffmpeg you are using, or what you changed the LIB???_N directives to so I can update the patch accordingly. I would like to make a pull request using this patch. It seems @Green-Sky made a pull request which resolves some or all of the private field issue, however, I feel as if the code in this pull request does not quite solve all the issues addressed here, noting that this patch also pulled in the deprecation issues among others...

[Green-Sky]

Sorry for the noise from an unrelated project. 😅
I adopted the simple change described in the OP with success.

[Korynkai]

@Green-Sky oh, silly me, I didn't realize that pull request was for qTox not freeswitch, my bad 🤣