Re: [libav-devel] [PATCH 1/3 v2] avutil/spherical: add functions to retrieve and request projection names

[Luca Barbato]

On 10/04/2017 22:31, Vittorio Giovara wrote:
> On Mon, Apr 10, 2017 at 1:43 PM, James Almer  wrote:
>>> Even so, what's the user going to do with knowing the fact that
>>> the video contains spherical metadata it can't parse correctly/fully
>>> support? So, I'm hesitant to add a new type for this uncertain
>>> behaviour, wouldn't you agree?
>>
>> No, because that's the whole point of avprobe, the dump.c code, and
>> other tools using the libraries: Informing the user of the contents
>> and characteristics of their files.
>> avconv and similar tools are what attempt to do something with them,
>> and for unknown projections they would do nothing.
> 
> I don't know, I feel like we're discussing about what kind of noise a
> tree would do falling in an empty forest :)
> I guess we should wait for other people to chime in, and solve this knot
> 

I'm happy as long the function does return an AVERROR on failure or
something easy to match, if I recall correctly Anton seems to be more
for not reporting possibly misleading data when it got discussed on IRC.
(Anton please confirm :))

I'd go with your initial patch amended to return AVERROR(ENOSYS) at
least for now.

lu
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 1/3 v2] avutil/spherical: add functions to retrieve and request projection names

[Vittorio Giovara]

On Mon, Apr 10, 2017 at 1:43 PM, James Almer  wrote:
>> Even so, what's the user going to do with knowing the fact that
>> the video contains spherical metadata it can't parse correctly/fully
>> support? So, I'm hesitant to add a new type for this uncertain
>> behaviour, wouldn't you agree?
>
> No, because that's the whole point of avprobe, the dump.c code, and
> other tools using the libraries: Informing the user of the contents
> and characteristics of their files.
> avconv and similar tools are what attempt to do something with them,
> and for unknown projections they would do nothing.

I don't know, I feel like we're discussing about what kind of noise a
tree would do falling in an empty forest :)
I guess we should wait for other people to chime in, and solve this knot
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 1/3 v2] avutil/spherical: add functions to retrieve and request projection names

[James Almer]

On 4/10/2017 1:20 PM, Vittorio Giovara wrote:
> On Mon, Apr 10, 2017 at 11:49 AM, James Almer  wrote:
>>> In my opinion it would be enough to return int and
>>> just AVERROR(EINVAL) in case it's not found. James would you be ok
>>> with that, or do you have any preference?
>>
>> IMO, PROJECTION_UNKNOWN is an interesting addition.
>>
>> Assume for example a projection we don't currently support shows up in
>> a file in the future, both the mov and matroska demuxers would ignore
>> it and report the file as having no spherical metadata whatsoever.
>> With PROJECTION_UNKNOWN the demuxers could report the presence of said
>> metadata (and the generic values yaw/pitch/roll) while clearly stating
>> it's unknown.
> 
> I mean, if it's unknown, no demuxer can parse correctly any field:
> nothing guarantees that future projections will keep y/p/r in the same
> place.

y/p/r are part of the prhd box in ISOBMFF. If that were to change,
the version field would be updated (We're currently skipping the
version field, so maybe we should add a check right now).
Projection specific boxes are currently cbmp, equi and mesh, which
makes me realize that we don't even need to think of hypothetical
future projections. For mesh files the unknown projection enum would
come in handy today.

In Matroska, y/p/r are standalone elements that may or may not be
present somewhere inside the Spherical master.

> Even so, what's the user going to do with knowing the fact that
> the video contains spherical metadata it can't parse correctly/fully
> support? So, I'm hesitant to add a new type for this uncertain
> behaviour, wouldn't you agree?

No, because that's the whole point of avprobe, the dump.c code, and
other tools using the libraries: Informing the user of the contents
and characteristics of their files.
avconv and similar tools are what attempt to do something with them,
and for unknown projections they would do nothing.

libavformat as it is right now will be aware that there's spherical
information in such files, but purposely pretend it's not there.
I personally think that's not the best behavior.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 1/3 v2] avutil/spherical: add functions to retrieve and request projection names

[Vittorio Giovara]

On Mon, Apr 10, 2017 at 11:49 AM, James Almer  wrote:
>> In my opinion it would be enough to return int and
>> just AVERROR(EINVAL) in case it's not found. James would you be ok
>> with that, or do you have any preference?
>
> IMO, PROJECTION_UNKNOWN is an interesting addition.
>
> Assume for example a projection we don't currently support shows up in
> a file in the future, both the mov and matroska demuxers would ignore
> it and report the file as having no spherical metadata whatsoever.
> With PROJECTION_UNKNOWN the demuxers could report the presence of said
> metadata (and the generic values yaw/pitch/roll) while clearly stating
> it's unknown.

I mean, if it's unknown, no demuxer can parse correctly any field:
nothing guarantees that future projections will keep y/p/r in the same
place. Even so, what's the user going to do with knowing the fact that
the video contains spherical metadata it can't parse correctly/fully
support? So, I'm hesitant to add a new type for this uncertain
behaviour, wouldn't you agree?
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 1/3 v2] avutil/spherical: add functions to retrieve and request projection names

[James Almer]

On 4/10/2017 10:56 AM, Vittorio Giovara wrote:
> On Fri, Apr 7, 2017 at 10:22 AM, Luca Barbato  wrote:
>> On 07/04/2017 16:09, James Almer wrote:
>>> +int av_spherical_from_name(const char *name);
>>
>> It can directly output the enum now, it is fine both ways to me.
> 
> Wait a minute, I don't think PROJECTION_UNKNOWN makes much sense to
> me, given the API and the spec it's based on, and -1 seems not really
> informative anyway.

The doxy in the original patch states -1 means not found. It's also
what the same function in stereo3d returns.

> In my opinion it would be enough to return int and
> just AVERROR(EINVAL) in case it's not found. James would you be ok
> with that, or do you have any preference?

IMO, PROJECTION_UNKNOWN is an interesting addition.

Assume for example a projection we don't currently support shows up in
a file in the future, both the mov and matroska demuxers would ignore
it and report the file as having no spherical metadata whatsoever.
With PROJECTION_UNKNOWN the demuxers could report the presence of said
metadata (and the generic values yaw/pitch/roll) while clearly stating
it's unknown.
Muxers would of course keep behaving the same and just not mux unknown
spherical metadata.

But in any case, either -1 or EINVAL are ok with me if you don't like
the above.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 1/3 v2] avutil/spherical: add functions to retrieve and request projection names

[Luca Barbato]

On 10/04/2017 15:56, Vittorio Giovara wrote:
> On Fri, Apr 7, 2017 at 10:22 AM, Luca Barbato  wrote:
>> On 07/04/2017 16:09, James Almer wrote:
>>> +int av_spherical_from_name(const char *name);
>>
>> It can directly output the enum now, it is fine both ways to me.
> 
> Wait a minute, I don't think PROJECTION_UNKNOWN makes much sense to
> me, given the API and the spec it's based on, and -1 seems not really
> informative anyway. In my opinion it would be enough to return int and
> just AVERROR(EINVAL) in case it's not found. James would you be ok
> with that, or do you have any preference?

I like that.

lu
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 1/3 v2] avutil/spherical: add functions to retrieve and request projection names

[Vittorio Giovara]

On Fri, Apr 7, 2017 at 10:22 AM, Luca Barbato  wrote:
> On 07/04/2017 16:09, James Almer wrote:
>> +int av_spherical_from_name(const char *name);
>
> It can directly output the enum now, it is fine both ways to me.

Wait a minute, I don't think PROJECTION_UNKNOWN makes much sense to
me, given the API and the spec it's based on, and -1 seems not really
informative anyway. In my opinion it would be enough to return int and
just AVERROR(EINVAL) in case it's not found. James would you be ok
with that, or do you have any preference?
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 1/3 v2] avutil/spherical: add functions to retrieve and request projection names

[Luca Barbato]

On 07/04/2017 16:09, James Almer wrote:
> +int av_spherical_from_name(const char *name);

It can directly output the enum now, it is fine both ways to me.

Thank you for amending it :)

lu
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel