[FFmpeg-devel] [PATCH 2/4] avformat/mpeg: Remove secondary packet for reading VobSub

[Andreas Rheinhardt]

Andreas Rheinhardt:
> Andreas Rheinhardt:
>> Andreas Rheinhardt:
>>> When vobsub_read_packet() reads a packet, it uses a dedicated AVPacket
>>> to get the subtitle timing and position from an FFDemuxSubtitlesQueue
>>> (which has been filled with this data during reading the idx file in
>>> vobsub_read_header); afterwards the actual subtitle data is read into
>>> the packet destined for output and the timing and position are copied
>>> to this packet. Afterwards, the local packet is unreferenced.
>>>
>>> This can be simplified: Simply use the output packet to get the timing
>>> and position from the FFDemuxSubtitlesQueue. The packet's size will be
>>> zero afterwards, so that it can be directly used to read the actual
>>> subtitle data. This makes copying the packet fields as well as
>>> unreferencing the local packet unecessary and also removes an instance
>>> of usage of sizeof(AVPacket) in libavformat.
>>>
>>> The only difference is that the returned packet will already be flagged
>>> as a keyframe. This currently only happens in compute_pkt_fields().
>>>
>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
>>> ---
>>>  libavformat/mpeg.c | 23 +++++++----------------
>>>  1 file changed, 7 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/libavformat/mpeg.c b/libavformat/mpeg.c
>>> index bd182e4429..7daa72f7ce 100644
>>> --- a/libavformat/mpeg.c
>>> +++ b/libavformat/mpeg.c
>>> @@ -912,7 +912,6 @@ static int vobsub_read_packet(AVFormatContext *s, AVPacket *pkt)
>>>      FFDemuxSubtitlesQueue *q;
>>>      AVIOContext *pb = vobsub->sub_ctx->pb;
>>>      int ret, psize, total_read = 0, i;
>>> -    AVPacket idx_pkt = { 0 };
>>>  
>>>      int64_t min_ts = INT64_MAX;
>>>      int sid = 0;
>>> @@ -927,24 +926,22 @@ static int vobsub_read_packet(AVFormatContext *s, AVPacket *pkt)
>>>          }
>>>      }
>>>      q = &vobsub->q[sid];
>>> -    ret = ff_subtitles_queue_read_packet(q, &idx_pkt);
>>> +    /* The returned packet will have size zero,
>>> +     * so that it can be directly used with av_grow_packet. */
>>> +    ret = ff_subtitles_queue_read_packet(q, pkt);
>>>      if (ret < 0)
>>>          return ret;
>>>  
>>>      /* compute maximum packet size using the next packet position. This is
>>>       * useful when the len in the header is non-sense */
>>>      if (q->current_sub_idx < q->nb_subs) {
>>> -        psize = q->subs[q->current_sub_idx].pos - idx_pkt.pos;
>>> +        psize = q->subs[q->current_sub_idx].pos - pkt->pos;
>>>      } else {
>>>          int64_t fsize = avio_size(pb);
>>> -        psize = fsize < 0 ? 0xffff : fsize - idx_pkt.pos;
>>> +        psize = fsize < 0 ? 0xffff : fsize - pkt->pos;
>>>      }
>>>  
>>> -    avio_seek(pb, idx_pkt.pos, SEEK_SET);
>>> -
>>> -    av_init_packet(pkt);
>>> -    pkt->size = 0;
>>> -    pkt->data = NULL;
>>> +    avio_seek(pb, pkt->pos, SEEK_SET);
>>>  
>>>      do {
>>>          int n, to_read, startcode;
>>> @@ -968,7 +965,7 @@ static int vobsub_read_packet(AVFormatContext *s, AVPacket *pkt)
>>>          total_read += pkt_size;
>>>  
>>>          /* the current chunk doesn't match the stream index (unlikely) */
>>> -        if ((startcode & 0x1f) != s->streams[idx_pkt.stream_index]->id)
>>> +        if ((startcode & 0x1f) != s->streams[pkt->stream_index]->id)
>>>              break;
>>>  
>>>          ret = av_grow_packet(pkt, to_read);
>>> @@ -980,16 +977,10 @@ static int vobsub_read_packet(AVFormatContext *s, AVPacket *pkt)
>>>              pkt->size -= to_read - n;
>>>      } while (total_read < psize);
>>>  
>>> -    pkt->pts = pkt->dts = idx_pkt.pts;
>>> -    pkt->pos = idx_pkt.pos;
>>> -    pkt->stream_index = idx_pkt.stream_index;
>>> -
>>> -    av_packet_unref(&idx_pkt);
>>>      return 0;
>>>  
>>>  fail:
>>>      av_packet_unref(pkt);
>>> -    av_packet_unref(&idx_pkt);
>>>      return ret;
>>>  }
>>>  
>>>
>> Ping.
>>
>> - Andreas
>>
> Another ping for the three unmerged patches of this patchset.
> 
> - Andreas
> 
Another ping for the two unmerged patches of this patchset; and also
for the other patch fixing uninitialized reads in the vobsub demuxer:
https://ffmpeg.org/pipermail/ffmpeg-devel/2019-October/251961.html

- Andreas