[MPlayer-dev-eng] [PATCH] deinterlace property fix
Alexander Strasser
eclipse7 at gmx.net
Mon Oct 22 17:11:18 CEST 2012
Hi Vicente,
Vicente Sendra wrote:
> --- 21/10/12, Alexander Strasser wrote:
[...]
> > Partially committed.
> >
> > I only committed the part relevant for the OSD
> > output. After
> > looking at other properties implemented in command.c I came
> > to
> > the conclusion that returning M_PROPERTY_ERROR is probably
> > not
> > the correct thing to do here. If it is a problem for you
> > please
> > explain me your use case.
> >
> > Thank you for your contribution!
>
> If you look at properties for brightness, contrast, hue, saturation, if you can not set or get them (not handled by any filter in the filter chain), it returns either value from set_video_colors if > 0 (just 1: M_PROPERTY_OK) or M_PROPERTY_UNAVAILABLE if no filter handles it.
>
> Maybe M_PROPERTY_ERROR is not the correct one, but, i think the right thing should be something like this:
>
> at getting:
> M_PROPERTY_OK if some vf handles it.
> M_PROPERTY_UNAVAILABLE if no filter handles it or assume it is disabled and return M_PROPERTY_OK (just as it is right now)
>
> at setting
> M_PROPERTY_OK if some vf handles it.
> M_PROPERTY_UNAVAILABLE if no filter handles it
> maybe M_PROPERTY_DISABLED if no filter handles it and we want to set it to true
>
> For me it's not logical to return M_PROPERTY_OK if the property was not set, it's more logical to return M_PROPERTY_UNAVAILABLE, so i can know if setting was successful, without having to request property one more time just to check if it was really set.
I agree with you that M_PROPERTY_UNAVAILABLE might be more correct and
better for handling on the client/caller side.
I will look into it. I have the complement part of the patch in a local
branch and can easily change and commit it. I just need to look if simply
returning M_PROPERTY_UNAVAILABLE if the return value is not CONTROL_OK is
enough or if there is another catch.
Having said that, my current opinion is that it would be an improvement
anyway.
Alexander
More information about the MPlayer-dev-eng
mailing list