[FFmpeg-devel] [PATCH] diracdec: add support for field coding

[Hendrik Leppkes]

On Tue, Jan 26, 2016 at 4:13 PM, Rostislav Pehlivanov
<atomnuker at gmail.com> wrote:
> On 26 January 2016 at 14:45, Carl Eugen Hoyos <cehoyos at ag.or.at> wrote:
>
>> Rostislav Pehlivanov <atomnuker <at> gmail.com> writes:
>>
>> > -            for (y = 0; y < p->height; y += 16) {
>>
>> > -                ff_spatial_idwt_slice2(&d, y+16); /* decode */
>>
>> > +            for (y = 0; y < p->height; y += 16)
>> > +                ff_spatial_idwt_slice2(&d, y+16);
>>
>> Sorry if it's just me but don't you agree that this is
>> much more readable without the unneeded changes?
>> (Apart from git blame.)
>>
>
> The space was removed so it doesn't waste a line.
> The decode comment was removed because it was out of place. What happens
> there is an inverse wavelet transform, but no decoding actually occurs
> since every single coefficient has already been read.
> The stride needs to get changed during interlacing (only on the output,
> transforms are still done on half height) so there's no getting away from
> actually changing the next line.
> Moved the s->diracdsp.put_signed_rect_clamped outside the loop as it should
> boost performance by a bit since it gets called only once instead of
> height/16 times.
> Really, the only single major change here is the
> s->diracdsp.put_signed_rect_clamped having its stride doubled on interlaced
> (required for this patch to work) and being moved outside the loop. And I'd
> say sending a separate patch for moving it outside the loop and removing a
> space and a wrong comment creates too much noise.
>
>
>> > +        if (s->field_coding && s->cur_field) { /* Copy the top field */
>> > +            int x, y;
>> > +            uint8_t *src = s->prev_field->data[comp];
>> > +            for (y = 0; y < p->height; y++) {
>> > +                for (x = 0; x < p->width*2; x++)
>> > +                    frame[p->stride*y*2 + x] = src[p->stride*y*2 + x];
>>
>> Can't you avoid this?
>>
>
> Don't know how/if it's even possible. I need to store the field somewhere
> and then copy it. I asked on IRC if it's even possible to reuse an AVFrame
> from a previous frame but got suggested to just set up a ref frame and
> unref it later, which is what I did.

Can't you just decode into your temporary frame without copying the field?
ie. first decode the top field in there, then the bottom field, and
then output the whole thing.

That would avoid a costly copy.

- Hendrik