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

[Carl Eugen Hoyos]

2018-05-19 23:48 GMT+02:00, James Almer <jamrial at gmail.com>:
> 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.

Reading the original submission above, isn't the whole point of the
patch to add public symbols to help downstream with real-world
issues?

Carl Eugen