[vlc-devel] [PATCH] Soft field repeat support for deinterlacer
Juha Jeronen
juha.jeronen at jyu.fi
Sat Feb 19 00:35:26 CET 2011
Hi,
Attached are the updated patches:
- Increase private_picture to 4, and
- Soft field repeat (repeat_pict) support for deinterlacer.
Laurent - please check whether these are now ok. The changes that we
agreed are now complete.
Additionally, I noticed that I had forgot to reset i_frame_offset in
Flush(). This is now fixed, too.
Detailed changelog:
- Patch split into two separate patches.
- Use a struct for the history stuff (last_dates et al.)
- Don't reuse "iend"
- Render as much as possible upon alloc failure (don't release
successfully allocated pictures)
- Check p_dst[i] != NULL in PTS generation
- Initialize pb_top_field_firsts properly
- Use a loop for setting b_progressive to true
- Use a loop for picture_Release() upon failure
- Reset i_frame_offset to 0 in Flush(), too
-J
On 02/17/11 21:48, Laurent Aimar wrote:
> Hi,
>
> On Wed, Feb 09, 2011 at 10:00:05PM +0200, Juha Jeronen wrote:
>> + /* Used for calculating field duration for framerate doublers */
>> + mtime_t pi_last_dates[LASTDATES_SIZE];
>> + int pi_last_nb_fields[LASTDATES_SIZE];
>> + bool pb_top_field_firsts[LASTDATES_SIZE];
> A structure to group the fields may ease a bit reading and/or the code. But
> it's not important.
>
>> + /* The last element corresponds to the current input frame. */
>> + p_sys->pi_last_dates[LASTDATES_SIZE-1] = p_pic->date;
>> + p_sys->pi_last_nb_fields[LASTDATES_SIZE-1] = p_pic->i_nb_fields;
>> + p_sys->pb_top_field_firsts[LASTDATES_SIZE-1] = p_pic->b_top_field_first;
> I would have put the current frame first, I think it would simplify a bit, but I
> am not sure. Not important at all.
>
>> + /* For framerate doublers, determine field duration and allocate output frames. */
>> + mtime_t i_field_dur = 0;
>> + if( p_sys->b_double_rate )
>> + {
>> + /* Calculate one field duration. */
>> + int i = 0;
>> + int iend = LASTDATES_SIZE-1;
>> + /* Find oldest valid logged date. Note: the current input frame doesn't count. */
>> + for( ; i < iend; i++ )
>> + if( p_sys->pi_last_dates[i] > VLC_TS_INVALID )
>> + break;
>> + if( i < iend )
>> + {
>> + /* Count how many fields the valid history entries (except the new frame) represent. */
>> + int i_fields_total = 0;
>> + for( int j = i ; j < iend; j++ )
>> + i_fields_total += p_sys->pi_last_nb_fields[j];
>> + /* One field took this long. */
>> + i_field_dur = (p_pic->date - p_sys->pi_last_dates[i]) / i_fields_total;
>> + }
>> + /* Note that we default to field duration 0 if it could not be determined.
>> + This behaves the same as the old code - leaving the extra output frame
>> + dates the same as p_pic->date if the last cached date was not valid.
>> + */
>> +
>> + /* Allocate output frames. */
>> + iend = i_nb_fields;
> Reusing iend with a different meaning is not great.
>> + if( i_nb_fields > DEINTERLACE_DST_SIZE )
>> + {
>> + /* Note that the effective buffer size depends also on the constant private_picture in vout_wrapper.c,
>> + as that determines the maximum number of output pictures filter_NewPicture() will successfully
>> + allocate for one input frame.
>> + */
>> + msg_Err( p_filter, "Framerate doubler: output buffer too small; fields = %d, buffer size = %d. Dropping the remaining fields.", i_nb_fields, DEINTERLACE_DST_SIZE );
>> + iend = DEINTERLACE_DST_SIZE;
>> + }
>> + bool b_alloc_success = true;
>> + for( int i = 1; i < iend ; ++i )
>> + {
>> + p_dst[i-1]->p_next =
>> + p_dst[i] = filter_NewPicture( p_filter );
>> + if( p_dst[i] )
>> + {
>> + picture_CopyProperties( p_dst[i], p_pic );
>> + }
>> + else
>> + {
>> + msg_Err( p_filter, "Framerate doubler: could not allocate output frame %d", i+1 );
>> + b_alloc_success = false;
>> + break; /* If this happens, the rest of the allocations aren't likely to work, either... */
>> + }
>> + }
>> + /* Clean up if alloc failed */
>> + if( !b_alloc_success )
>> + {
>> + for( int i = 1; i < iend; ++i )
>> + {
>> + /* Here we need the fact that any unused p_dst have been initialized to NULL. */
>> + if( p_dst[i] )
>> + {
>> + picture_Release( p_dst[i] );
>> + p_dst[i] = NULL;
>> + }
>> + }
>> + }
> I don't think it is worth it to release successfully allocated picture. Because
> if you don't, then you output as much as possible, which I think is better (and
> simpler).
>
>> + /* Set output timestamps, if the algorithm didn't request CUSTOM_PTS for this frame */
>> + assert( i_frame_offset <= LASTDATES_SIZE || i_frame_offset == CUSTOM_PTS );
>> + if( i_frame_offset != CUSTOM_PTS )
>> + {
>> + mtime_t i_base_pts = p_sys->pi_last_dates[i_idx_to_lasts];
>> +
>> + p_dst[0]->date = i_base_pts;
>> + if( p_sys->b_double_rate )
>> + {
>> + for( int i = 1; i < i_nb_fields; ++i )
>> + {
>> + /* XXX it's not really good especially for the first picture, but
>> + * I don't think that delaying by one frame is worth it */
>> + if( i_base_pts > VLC_TS_INVALID )
>> + p_dst[i]->date = i_base_pts + i * i_field_dur;
>> + else
>> + p_dst[i]->date = VLC_TS_INVALID;
> I think p_dst[i] can be NULL in case of allocation failures.
>> + }
>> + }
>> + }
>> +
>> p_dst[0]->b_progressive = true;
>> if( p_dst[1] )
>> p_dst[1]->b_progressive = true;
>> + if( p_dst[2] )
>> + p_dst[2]->b_progressive = true;
>>
>> picture_Release( p_pic );
>> return p_dst[0];
>> @@ -1622,6 +1828,8 @@ drop:
>> picture_Release( p_dst[0] );
>> if( p_dst[1] )
>> picture_Release( p_dst[1] );
>> + if( p_dst[2] )
>> + picture_Release( p_dst[2] );
>> picture_Release( p_pic );
>> return NULL;
>> }
>> @@ -1630,7 +1838,11 @@ static void Flush( filter_t *p_filter )
>> {
>> filter_sys_t *p_sys = p_filter->p_sys;
>>
>> - p_sys->i_last_date = VLC_TS_INVALID;
>> + for( int i = 0; i < LASTDATES_SIZE; i++ )
>> + {
>> + p_sys->pi_last_dates[i] = VLC_TS_INVALID;
>> + p_sys->pi_last_nb_fields[i] = 2;
>> + }
>> for( int i = 0; i < HISTORY_SIZE; i++ )
>> {
>> if( p_sys->pp_history[i] )
>> @@ -1669,7 +1881,12 @@ static int Open( vlc_object_t *p_this )
>> p_sys->i_mode = DEINTERLACE_BLEND;
>> p_sys->b_double_rate = false;
>> p_sys->b_half_height = true;
>> - p_sys->i_last_date = VLC_TS_INVALID;
>> + for( int i = 0; i < LASTDATES_SIZE; i++ )
>> + {
>> + p_sys->pi_last_dates[i] = VLC_TS_INVALID;
>> + p_sys->pi_last_nb_fields[i] = 2;
>> + }
>> + p_sys->i_frame_offset = 0; /* start with default value (first-ever frame cannot have offset) */
>> for( int i = 0; i < HISTORY_SIZE; i++ )
>> p_sys->pp_history[i] = NULL;
>> diff --git a/src/video_output/vout_wrapper.c b/src/video_output/vout_wrapper.c
>> index 8ff36b8..2015dc4 100644
>> --- a/src/video_output/vout_wrapper.c
>> +++ b/src/video_output/vout_wrapper.c
>> @@ -134,7 +134,7 @@ int vout_InitWrapper(vout_thread_t *vout)
>>
>> sys->display.use_dr = !vout_IsDisplayFiltered(vd);
>> const bool allow_dr = !vd->info.has_pictures_invalid && sys->display.use_dr;
>> - const unsigned private_picture = 3; /* XXX 2 for filter, 1 for SPU */
>> + const unsigned private_picture = 4; /* XXX 3 for filter, 1 for SPU */
>> const unsigned decoder_picture = 1 + sys->dpb_size;
>> const unsigned kept_picture = 1; /* last displayed picture */
>> const unsigned reserved_picture = DISPLAY_PICTURE_COUNT +
> I think I would prefer it to be in an independant patch. There are related
> but it can be used on its own (I'm toying on a filter which need it too
> for example).
>
> Regards,
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Increase-private_picture-to-4-to-accommodate-filters.patch
Type: text/x-patch
Size: 1092 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20110219/de1e33e5/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Soft-field-repeat-support-for-deinterlacer.patch
Type: text/x-patch
Size: 19108 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20110219/de1e33e5/attachment-0001.bin>
More information about the vlc-devel
mailing list