| Message ID | c08ed165-636b-44fa-80f8-ba91f294ebc0@aitek.it |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/avformat: Added codec_name to AVCodecContext and AVCodecParameters | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/configure_loongarch64 | warning | Failed to apply patch |
| andriy/configure_x86 | warning | Failed to apply patch |
On 03/07/2024 10:02, Hendrik Leppkes wrote: > On Wed, Jul 3, 2024 at 9:48 AM Bernardo Pilarz via ffmpeg-devel > <ffmpeg-devel@ffmpeg.org> wrote: >> Added the codec_name field, in which the unprocessed, not-interpreted codec >> name is stored. >> This is useful when codecs that are not handled by the libav >> (i.e. AV_CODEC_ID_NONE) are encountered, since the application might still >> want to handle them. >> Having this field allows the application to determine the codec type. >> >> As of this commit, the codec_name field is only filled when opening an RTSP >> stream, during the parsing of the SDP. >> >> Signed-off-by: bpilarz <bernardo.pilarz@aitek.it> >> --- >> libavcodec/avcodec.h | 10 ++++++++++ >> libavcodec/codec_par.c | 19 +++++++++++++++++++ >> libavcodec/codec_par.h | 10 ++++++++++ >> libavcodec/options.c | 1 + >> libavformat/rtsp.c | 5 +++++ >> 5 files changed, 45 insertions(+) >> >> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h >> index 2da63c87ea..464b4078fc 100644 >> --- a/libavcodec/avcodec.h >> +++ b/libavcodec/avcodec.h >> @@ -451,6 +451,16 @@ typedef struct AVCodecContext { >> int log_level_offset; >> >> enum AVMediaType codec_type; /* see AVMEDIA_TYPE_xxx */ >> + /** >> + * Generic codec name of the encoded data. >> + * >> + * Null-terminated string, can be NULL. >> + * Contents may vary depending on the source of the data stream. >> + * This is typically the string that's interpreted to determine >> 'codec_id'. >> + * Must be allocated with av_malloc() or av_strdup() and will be >> freed by >> + * avcodec_free_context(). >> + */ >> + char *codec_name; >> const struct AVCodec *codec; >> enum AVCodecID codec_id; /* see AV_CODEC_ID_xxx */ >> > Adding a new field here is an ABI break, it would need to go at the > end of the struct. > > In general, I feel like this might be better served to go into > metadata though, especially as very few containers have a string codec > identifier to begin with. > > - Hendrik > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > I would be very glad to do it the right way, but I need some guidance since this is the first time I try to contribute to FFmpeg. The problem that I am trying to solve is receiving metadata from an RTSP stream (in example, ONVIF metadata identified by the codec name 'vcd.onvif.metadata'). This is data that the application will want to handle on its own (and not through FFmpeg). Can you guide me on how to do this properly? Thanks, Bernardo
Quoting Bernardo Pilarz via ffmpeg-devel (2024-07-03 10:10:15) > >> + char *codec_name; > >> const struct AVCodec *codec; > >> enum AVCodecID codec_id; /* see AV_CODEC_ID_xxx */ > >> > > Adding a new field here is an ABI break, it would need to go at the > > end of the struct. > > > > In general, I feel like this might be better served to go into > > metadata though, especially as very few containers have a string codec > > identifier to begin with. > > I would be very glad to do it the right way, but I need some guidance > since this is the first time I try to contribute to FFmpeg. > > The problem that I am trying to solve is receiving metadata from an RTSP > stream (in example, ONVIF metadata identified by the codec name > 'vcd.onvif.metadata'). > This is data that the application will want to handle on its own (and > not through FFmpeg). > > Can you guide me on how to do this properly? First of all, this should definitely NOT go into AVCodecContext, since the entire premise is that we have no codec ID and hence no decoders or encoders, so a codec context makes no sense. Next, the idea that you can define a generic container-independent "codec name" sounds dubious to me, as they are not universally standardized. You'd either have to define precisely what can the API caller expect in there, or make this a private AV_OPT_FLAG_EXPORT option of your demuxer. I'd suggest the second, as it's much simpler, though IIRC we do not yet support per-stream private options.
On 03/07/2024 11:10, Anton Khirnov wrote: > Quoting Bernardo Pilarz via ffmpeg-devel (2024-07-03 10:10:15) >>>> + char *codec_name; >>>> const struct AVCodec *codec; >>>> enum AVCodecID codec_id; /* see AV_CODEC_ID_xxx */ >>>> >>> Adding a new field here is an ABI break, it would need to go at the >>> end of the struct. >>> >>> In general, I feel like this might be better served to go into >>> metadata though, especially as very few containers have a string codec >>> identifier to begin with. >> I would be very glad to do it the right way, but I need some guidance >> since this is the first time I try to contribute to FFmpeg. >> >> The problem that I am trying to solve is receiving metadata from an RTSP >> stream (in example, ONVIF metadata identified by the codec name >> 'vcd.onvif.metadata'). >> This is data that the application will want to handle on its own (and >> not through FFmpeg). >> >> Can you guide me on how to do this properly? > First of all, this should definitely NOT go into AVCodecContext, since > the entire premise is that we have no codec ID and hence no decoders or > encoders, so a codec context makes no sense. > > Next, the idea that you can define a generic container-independent > "codec name" sounds dubious to me, as they are not universally > standardized. You'd either have to define precisely what can the API > caller expect in there, or make this a private AV_OPT_FLAG_EXPORT option > of your demuxer. I'd suggest the second, as it's much simpler, though > IIRC we do not yet support per-stream private options. > I ended up adding it to the AVCodecContext because 'avformat_find_stream_info()' invokes 'avcodec_parameters_to_context()' followed by 'avcodec_parameters_from_context()', thereby overwriting the entire 'codecpar' field of each stream in my format context. This also means that an AVCodecContext is in fact created even for streams with no codec ID. It's not clear to me how I can use AV_OPT_FLAG_EXPORT (it's my fault because I don't know the library so well). It might very well be the solution I am looking for, but I need some help in understanding how to implement it. The basic problem is I need to get the "codec name" information that is available in 'sdp_parse_rtpmap()' and is completely lost when the codec is not supported by FFmpeg. By the way, we are talking of AVMEDIA_TYPE_DATA streams, and codecs that are potentially custom/not widespread and therefore should probably be handled by the application rather than by ffmpeg itself. P.S.: I am currently only using FFmpeg to grab an RTSP stream. The audio and video streams might very well get decoded using FFmpeg later on, but so far I am only concerned with getting the compressed packets and being able to read their declared codec names.
On 03/07/2024 11:56, Bernardo Pilarz via ffmpeg-devel wrote: > On 03/07/2024 11:10, Anton Khirnov wrote: >> Quoting Bernardo Pilarz via ffmpeg-devel (2024-07-03 10:10:15) >>>>> + char *codec_name; >>>>> const struct AVCodec *codec; >>>>> enum AVCodecID codec_id; /* see AV_CODEC_ID_xxx */ >>>>> >>>> Adding a new field here is an ABI break, it would need to go at the >>>> end of the struct. >>>> >>>> In general, I feel like this might be better served to go into >>>> metadata though, especially as very few containers have a string codec >>>> identifier to begin with. >>> I would be very glad to do it the right way, but I need some guidance >>> since this is the first time I try to contribute to FFmpeg. >>> >>> The problem that I am trying to solve is receiving metadata from an >>> RTSP >>> stream (in example, ONVIF metadata identified by the codec name >>> 'vcd.onvif.metadata'). >>> This is data that the application will want to handle on its own (and >>> not through FFmpeg). >>> >>> Can you guide me on how to do this properly? >> First of all, this should definitely NOT go into AVCodecContext, since >> the entire premise is that we have no codec ID and hence no decoders or >> encoders, so a codec context makes no sense. >> >> Next, the idea that you can define a generic container-independent >> "codec name" sounds dubious to me, as they are not universally >> standardized. You'd either have to define precisely what can the API >> caller expect in there, or make this a private AV_OPT_FLAG_EXPORT option >> of your demuxer. I'd suggest the second, as it's much simpler, though >> IIRC we do not yet support per-stream private options. >> > I ended up adding it to the AVCodecContext because > 'avformat_find_stream_info()' invokes > 'avcodec_parameters_to_context()' followed by > 'avcodec_parameters_from_context()', thereby overwriting the entire > 'codecpar' field of each stream in my format context. > This also means that an AVCodecContext is in fact created even for > streams with no codec ID. > > It's not clear to me how I can use AV_OPT_FLAG_EXPORT (it's my fault > because I don't know the library so well). > It might very well be the solution I am looking for, but I need some > help in understanding how to implement it. > > The basic problem is I need to get the "codec name" information that > is available in 'sdp_parse_rtpmap()' and is completely lost when the > codec is not supported by FFmpeg. > By the way, we are talking of AVMEDIA_TYPE_DATA streams, and codecs > that are potentially custom/not widespread and therefore should > probably be handled by the application rather than by ffmpeg itself. > > P.S.: I am currently only using FFmpeg to grab an RTSP stream. The > audio and video streams might very well get decoded using FFmpeg later > on, but so far I am only concerned with getting the compressed packets > and being able to read their declared codec names. > > How about putting this information in the stream's 'codecpar.coded_side_data'?
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h index 2da63c87ea..464b4078fc 100644 --- a/libavcodec/avcodec.h +++ b/libavcodec/avcodec.h @@ -451,6 +451,16 @@ typedef struct AVCodecContext { int log_level_offset; enum AVMediaType codec_type; /* see AVMEDIA_TYPE_xxx */ + /** + * Generic codec name of the encoded data. + * + * Null-terminated string, can be NULL. + * Contents may vary depending on the source of the data stream. + * This is typically the string that's interpreted to determine 'codec_id'. + * Must be allocated with av_malloc() or av_strdup() and will be freed by + * avcodec_free_context(). + */ + char *codec_name; const struct AVCodec *codec; enum AVCodecID codec_id; /* see AV_CODEC_ID_xxx */ diff --git a/libavcodec/codec_par.c b/libavcodec/codec_par.c index 790ea01d10..27d9880b53 100644 --- a/libavcodec/codec_par.c +++ b/libavcodec/codec_par.c @@ -31,6 +31,7 @@ static void codec_parameters_reset(AVCodecParameters *par) { + av_freep(&par->codec_name); av_freep(&par->extradata); av_channel_layout_uninit(&par->ch_layout); av_packet_side_data_free(&par->coded_side_data, &par->nb_coded_side_data); @@ -110,6 +111,13 @@ int avcodec_parameters_copy(AVCodecParameters *dst, const AVCodecParameters *src codec_parameters_reset(dst); memcpy(dst, src, sizeof(*dst)); + dst->codec_name = NULL; + if (src->codec_name) { + dst->codec_name = av_strdup(src->codec_name); + if (!dst->codec_name) + return AVERROR(ENOMEM); + } + dst->ch_layout = (AVChannelLayout){0}; dst->extradata = NULL; dst->extradata_size = 0; @@ -142,6 +150,11 @@ int avcodec_parameters_from_context(AVCodecParameters *par, codec_parameters_reset(par); par->codec_type = codec->codec_type; + if (codec->codec_name) { + par->codec_name = av_strdup(codec->codec_name); + if (!par->codec_name) + return AVERROR(ENOMEM); + } par->codec_id = codec->codec_id; par->codec_tag = codec->codec_tag; @@ -206,6 +219,12 @@ int avcodec_parameters_to_context(AVCodecContext *codec, int ret; codec->codec_type = par->codec_type; + av_freep(&codec->codec_name); + if (par->codec_name) { + codec->codec_name = av_strdup(par->codec_name); + if (!codec->codec_name) + return AVERROR(ENOMEM); + } codec->codec_id = par->codec_id; codec->codec_tag = par->codec_tag; diff --git a/libavcodec/codec_par.h b/libavcodec/codec_par.h index f4b9bb5c06..196ef1fc66 100644 --- a/libavcodec/codec_par.h +++ b/libavcodec/codec_par.h @@ -49,6 +49,16 @@ typedef struct AVCodecParameters { * General type of the encoded data. */ enum AVMediaType codec_type; + /** + * Generic codec name of the encoded data. + * + * Null-terminated string, can be NULL. + * Contents may vary depending on the source of the data stream. + * This is typically the string that's interpreted to determine 'codec_id'. + * Must be allocated with av_malloc() or av_strdup() and will be freed by + * avcodec_parameters_free(). + */ + char *codec_name; /** * Specific type of the encoded data (the codec used). */ diff --git a/libavcodec/options.c b/libavcodec/options.c index f60c41bdc3..c107f4efbd 100644 --- a/libavcodec/options.c +++ b/libavcodec/options.c @@ -170,6 +170,7 @@ void avcodec_free_context(AVCodecContext **pavctx) ff_codec_close(avctx); + av_freep(&avctx->codec_name); av_freep(&avctx->extradata); av_freep(&avctx->subtitle_header); av_freep(&avctx->intra_matrix); diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c index db78735c7a..56363caeda 100644 --- a/libavformat/rtsp.c +++ b/libavformat/rtsp.c @@ -317,6 +317,11 @@ static int sdp_parse_rtpmap(AVFormatContext *s, par->codec_id = ff_rtp_codec_id(buf, par->codec_type); } + /* Copy the codec name, regardless of whether we can handle it. + * Applications might handle unknown codecs on their own. */ + av_freep(&par->codec_name); + par->codec_name = av_strdup(buf); + desc = avcodec_descriptor_get(par->codec_id); if (desc && desc->name) c_name = desc->name;
Added the codec_name field, in which the unprocessed, not-interpreted codec name is stored. This is useful when codecs that are not handled by the libav (i.e. AV_CODEC_ID_NONE) are encountered, since the application might still want to handle them. Having this field allows the application to determine the codec type. As of this commit, the codec_name field is only filled when opening an RTSP stream, during the parsing of the SDP. Signed-off-by: bpilarz <bernardo.pilarz@aitek.it> --- libavcodec/avcodec.h | 10 ++++++++++ libavcodec/codec_par.c | 19 +++++++++++++++++++ libavcodec/codec_par.h | 10 ++++++++++ libavcodec/options.c | 1 + libavformat/rtsp.c | 5 +++++ 5 files changed, 45 insertions(+)