[FFmpeg-devel] [PATCH] avformat: move internal stream related functions and structs to a separate header

[James Almer]

On 1/24/2024 12:14 PM, Andreas Rheinhardt wrote:
> James Almer:
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>>   libavformat/Makefile          |   1 +
>>   libavformat/avformat.c        | 171 ----------------
>>   libavformat/demux.h           |  29 ---
>>   libavformat/internal.h        | 319 +---------------------------
>>   libavformat/stream.c          | 196 ++++++++++++++++++
>>   libavformat/stream_internal.h | 376 ++++++++++++++++++++++++++++++++++
>>   6 files changed, 574 insertions(+), 518 deletions(-)
>>   create mode 100644 libavformat/stream.c
>>   create mode 100644 libavformat/stream_internal.h
>>
>> diff --git a/libavformat/internal.h b/libavformat/internal.h
>> index f93832b3c4..f97ebc818f 100644
>> --- a/libavformat/internal.h
>> +++ b/libavformat/internal.h
>> @@ -26,6 +26,7 @@
>>   #include "libavcodec/packet_internal.h"
>>   
>>   #include "avformat.h"
>> +#include "stream_internal.h"
> 
> You seem to have included the new header in order to avoid checking
> which of the internal.h inclusions should be replaced by
> stream_internal.h. This basically defeats the point of adding a new
> header (honestly, I don't really get the point of splitting this off
> anyway: avformat.c is not that big).

Anton suggested to split the stream related functions into their own 
file. But agree there's not a lot to gain.

> 
>>   
>>   #define MAX_URL_SIZE 4096
>>   
> 
> ...
> 
>> diff --git a/libavformat/stream_internal.h b/libavformat/stream_internal.h
>> new file mode 100644
>> index 0000000000..9759f30e3f
>> --- /dev/null
>> +++ b/libavformat/stream_internal.h
>> @@ -0,0 +1,376 @@
>> +/*
>> + * Stream related functions and structs
>> + *
>> + * This file is part of FFmpeg.
>> + *
>> + * FFmpeg is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * FFmpeg is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with FFmpeg; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>> + */
>> +
>> +#ifndef AVFORMAT_STREAM_INTERNAL_H
>> +#define AVFORMAT_STREAM_INTERNAL_H
>> +
>> +#include <stdint.h>
>> +
>> +#include "libavcodec/bsf.h"
>> +#include "libavcodec/packet_internal.h"
>> +#include "libavcodec/parser.h"
> 
> Including bsf.h and parser.h is unnecessary (the structs here don't need
> the complete types and almost no file including this needs them either).
> 
>> +#include "avformat.h"
>> +
>> +/**
>> + * Fractional numbers for exact pts handling
>> + *
>> + * The exact value of the fractional number is: 'val + num / den'.
>> + * num is assumed to be 0 <= num < den.
>> + */
>> +typedef struct FFFrac {
>> +    int64_t val, num, den;
>> +} FFFrac;
>> +
>> +#define MAX_STD_TIMEBASES (30*12+30+3+6)
>> +typedef struct FFStreamInfo {
>> +    int64_t last_dts;
>> +    int64_t duration_gcd;
>> +    int duration_count;
>> +    int64_t rfps_duration_sum;
>> +    double (*duration_error)[2][MAX_STD_TIMEBASES];
>> +    int64_t codec_info_duration;
>> +    int64_t codec_info_duration_fields;
>> +    int frame_delay_evidence;
>> +
>> +    /**
>> +     * 0  -> decoder has not been searched for yet.
>> +     * >0 -> decoder found
>> +     * <0 -> decoder with codec_id == -found_decoder has not been found
>> +     */
>> +    int found_decoder;
>> +
>> +    int64_t last_duration;
>> +
>> +    /**
>> +     * Those are used for average framerate estimation.
>> +     */
>> +    int64_t fps_first_dts;
>> +    int     fps_first_dts_idx;
>> +    int64_t fps_last_dts;
>> +    int     fps_last_dts_idx;
>> +} FFStreamInfo;
> 
> Moving FFStreamInfo to this header makes it accessible to muxers,
> although this is supposed to be a demuxer-only struct.
> (Didn't you send this patch once before?)

Ah, guess i did. I did get a sense of deja vu when i wrote this.
I couldn't find anything like this in my local repo, so i might have 
lost the commit after sending the patch and ended up writing it again.