[libav-devel] [PATCH] pgssubdec: handle more complex PGS scenarios

John Stebbins stebbins at jetheaddev.com
Wed Jun 18 22:38:19 CEST 2014


On 06/17/2014 10:43 PM, Anton Khirnov wrote:
> On Tue, 17 Jun 2014 13:25:31 -0700, John Stebbins <stebbins at jetheaddev.com> wrote:
>> On 06/17/2014 01:18 PM, John Stebbins wrote:
>>> On 06/17/2014 12:29 PM, Anton Khirnov wrote:
>>>> On Fri, 13 Jun 2014 07:56:23 -0700, John Stebbins <stebbins at jetheaddev.com> wrote:
>>>>> Add ability to handle multiple palettes and objects simultaneously.
>>>>> Each simultaneous object is given its own AVSubtitleRect.
>>>>> Note that there can be up to 64 currently valid objects, but only
>>>>> 2 at any one time can be "presented".
>>>>> ---
>>>>> @@ -369,43 +489,72 @@ static int display_end_segment(AVCodecContext *avctx, void *data,
>>>>>      memset(sub, 0, sizeof(*sub));
>>>>>      sub->pts = ctx->presentation.pts;
>>>>>  
>>>>> -    // Blank if last object_number was 0.
>>>>> +    // Blank if last object_count was 0.
>>>>>      // Note that this may be wrong for more complex subtitles.
>>>>> -    if (!ctx->presentation.object_number)
>>>>> +    if (!ctx->presentation.object_count)
>>>>>          return 1;
>>>>>      sub->start_display_time = 0;
>>>>>      sub->end_display_time   = 20000;
>>>>>      sub->format             = 0;
>>>>>  
>>>>> -    sub->rects     = av_mallocz(sizeof(*sub->rects));
>>>>> -    sub->rects[0]  = av_mallocz(sizeof(*sub->rects[0]));
>>>>> -    sub->num_rects = 1;
>>>>> -
>>>>> -    if (ctx->presentation.composition_flag & 0x40)
>>>>> -        sub->rects[0]->flags |= AV_SUBTITLE_FLAG_FORCED;
>>>>> +    sub->num_rects = ctx->presentation.object_count;
>>>>> +    sub->rects     = av_mallocz(sizeof(*sub->rects) * sub->num_rects);
>>>>> +    if (!sub->rects) {
>>>>> +        return -1;
>>>> >From looking at the code, it seems that returning negative numbers from this
>>>> function won't work as expected.
>>>> Also, shouldn't you free sub->rects on failure here?
>>>>
>>> Ah, yes.  There is no way to indicate an error here.  Can only return that we have a sub or we don't.  Looking closer, I
>>> see that the return on failure of decode_rle would leak memory since the sub is partially initialized, but would not be
>>> freed.  I will propose a fix for that as well in an updated patch.
>>>
>>>
>> Forgot to mention, there are cases where it would be desirable to keep sub->rects even when an error has been
>> encountered since there may be more than one rect in the sub and one may be valid even though an error is encountered in
>> another.
>>
>> If we want a total abort, I would also need to run through a loop and delete each rect that has been allocated.  My
>> thought is I could just set w and h to 0 (empty rect) when an error is encountered and continue (if recoverable) or
>> return 1 (if not recoverable).
>>
> The canonical way is to check err_recognition. If it's set to explode, then
> return failure. Otherwise try to continue if possible.
>

Ok, I'll clean up error handling so that a proper error code can be propagated back to the caller.

-- 
John      GnuPG fingerprint: D0EC B3DB C372 D1F1 0B01  83F0 49F1 D7B2 60D4 D0F7


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <http://lists.libav.org/pipermail/libav-devel/attachments/20140618/c9b79931/attachment-0001.sig>


More information about the libav-devel mailing list