[vlc-devel] [PATCH] Adds Support for Intel's QuickSync Video encoder

[Ilkka Ollakka]

On Wed, May 08, 2013 at 10:04:20PM +0200, Julien 'Lta' BALLET wrote:
> Hi,

Hi,

> I gotta warn you, there's a small spinlock in the encode loop which cannot
> be removed unless you wanna loose frames (and maybe create a passage to
> another dimension full of ponies), but it's only triggered once every 2-5s.

I don't comment on those that funman already commented on, atleast not
on purpose.

> +    add_bool(SOUT_CFG_PREFIX "software", false, SW_IMPL_TEXT, SW_IMPL_LONGTEXT, true)

Any reason software couldn't be enabled by default? 

> +static uint64_t qsv_params_get_value(const char *const *text,
> +                                     const int const *list,
> +                                     size_t size, char *sel)
> +{
> +    for (size_t i = 0; i < size; i++)
> +        if (!strcmp(sel, text[i]))
> +            return list[i];
> +
> +    // The default value is always the first item in the array. So if not found, use it.
> +    return 0;

Shouldnt this be list[0] ?

Also you seem to use this with size calculation of
sizeof(level_h264_list) / sizeof(int), so it could clearer to just move
that size calculation inside of this function.

> +    } else if (sts > MFX_ERR_NONE)
> +        msg_Dbg(enc, "Video encoder initialization warning (%d).", sts);

maybe msg_Warn in here?

> +static block_t *Encode(encoder_t *this, picture_t *pic)

> +    sts = MFXVideoENCODE_EncodeFrameAsync(sys->session, 0, frame, &task->bs, &task->syncp);
> +    while (sts == MFX_WRN_DEVICE_BUSY) {
> +        msg_Info(enc, "Device is busy, let's wait and retry");
> +        msleep(QSV_SPINLOCK_TIME);
> +        sts = MFXVideoENCODE_EncodeFrameAsync(sys->session, 0, frame, &task->bs, &task->syncp);
> +    }

Are you sure about that msleep time (20000) ? I would prefer same
aproach that funman mentioned to just store the task and retry on next
run instead of blocking here every ~100 frames (2-5 secs according to
you).

-- 
Ilkka Ollakka
"The road to hell is paved with melting snowballs."
		-- Larry Wall in  <1992Jul2.222039.26476 at netlabs.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20130510/72f67602/attachment.sig>