[FFmpeg-devel] [PATCH v4 1/3] avformat: add fields to AVProgram/AVStream for PMT change tracking

[James Almer]

On 5/19/2018 6:31 PM, Michael Niedermayer wrote:
> On Fri, May 18, 2018 at 11:15:04AM -0700, Aman Gupta wrote:
>> From: Aman Gupta <aman at tmm1.net>
>>
>> These fields will allow the mpegts demuxer to expose details about
>> the PMT/program which created the AVProgram and its AVStreams.
>>
>> In mpegts, a PMT which advertises streams has a version number
>> which can be incremented at any time. When the version changes,
>> the pids which correspond to each of it's streams can also change.
>>
>> Since ffmpeg creates a new AVStream per pid by default, an API user
>> needs the ability to (a) detect when the PMT changed, and (b) tell
>> which AVStream were added to replace earlier streams.
>>
>> This has been a long-standing issue with ffmpeg's handling of mpegts
>> streams with PMT changes, and I found two related patches in the wild
>> that attempt to solve the same problem.
>>
>> The first is in MythTV's ffmpeg fork, where they added a
>> void (*streams_changed)(void*); to AVFormatContext and call it from
>> their fork of the mpegts demuxer whenever the PMT changes.
>>
>> The second was proposed by XBMC in
>> https://ffmpeg.org/pipermail/ffmpeg-devel/2012-December/135036.html,
>> where they created a new AVMEDIA_TYPE_DATA stream with id=0 and
>> attempted to send packets to it whenever the PMT changed.
>>
>> Signed-off-by: Aman Gupta <aman at tmm1.net>
>> ---
>>  doc/APIchanges         | 4 ++++
>>  libavformat/avformat.h | 8 ++++++++
>>  libavformat/utils.c    | 1 +
>>  libavformat/version.h  | 2 +-
>>  4 files changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/doc/APIchanges b/doc/APIchanges
>> index befa58c84a..a592073ca5 100644
>> --- a/doc/APIchanges
>> +++ b/doc/APIchanges
>> @@ -15,6 +15,10 @@ libavutil:     2017-10-21
>>  
>>  API changes, most recent first:
>>  
>> +2018-05-xx - xxxxxxxxxx - lavf 58.15.100 - avformat.h
>> +  Add pmt_version field to AVProgram
>> +  Add program_num, pmt_version, pmt_stream_idx to AVStream
>> +
>>  2018-05-xx - xxxxxxxxxx - lavf 58.14.100 - avformat.h
>>    Add AV_DISPOSITION_STILL_IMAGE
>>  
>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>> index 6dce88fad5..ade918f99c 100644
>> --- a/libavformat/avformat.h
>> +++ b/libavformat/avformat.h
>> @@ -1103,6 +1103,13 @@ typedef struct AVStream {
>>       */
>>      int stream_identifier;
>>  
>> +    /**
>> +     * Details of the MPEG-TS program which created this stream.
>> +     */
>> +    int program_num;
>> +    int pmt_version;
>> +    int pmt_stream_idx;
>> +
>>      int64_t interleaver_chunk_size;
>>      int64_t interleaver_chunk_duration;
>>  
> 
> These are added below the "All fields below this line are not part of the public API."
> This contradicts the addition to APIChanges

If these don't need to be accessed by the API user then I'd rather keep
them here than moving them to the public section. Just remove the
APIChanges entry.
Same thing with the AVStream pmt_version field. If direct access is not
needed for it, then it should be moved below the public notice.

These fields seem pretty specific to one demuxer so ideally they
shouldn't be public in case changes to them and the implementation are
ever needed.