Proof of concept using libvpx's y4minput.c

Proof of concept using libvpx's y4minput.c

This prevents reinventing the wheel in https://codereview.webrtc.org/2515253004

[kjellander_webrtc, 2016-11-30 07:50:42]

Patchset #1 (id:1) has been deleted

[kjellander_webrtc, 2016-11-30 07:51:58]

Patchset #1 (id:20001) has been deleted

[charujain, 2016-11-30 09:58:23]

charujain@webrtc.org changed reviewers:
+ charujain@webrtc.org

[charujain, 2016-11-30 09:58:23]

Great. Thanks for creating this. Just one question on this.

https://codereview.webrtc.org/2540893003/diff/60001/webrtc/tools/frame_analyz...
File webrtc/tools/frame_analyzer/reference_less_video_analysis_lib.cc (right):

https://codereview.webrtc.org/2540893003/diff/60001/webrtc/tools/frame_analyz...
webrtc/tools/frame_analyzer/reference_less_video_analysis_lib.cc:29:
y4m_input_open(&y4m, input_file, NULL, 0, 0);
We are using this function since this internally calls "y4m_parse_tags" which is
what we need. But since that is not directly exposed in library you are calling
this. But is this a good practise? Since this function does lot more than just
parsing.

[kjellander_webrtc, 2016-11-30 10:00:01]

On 2016/11/30 09:58:23, charujain wrote:
> Great. Thanks for creating this. Just one question on this.
> 
>
https://codereview.webrtc.org/2540893003/diff/60001/webrtc/tools/frame_analyz...
> File webrtc/tools/frame_analyzer/reference_less_video_analysis_lib.cc (right):
> 
>
https://codereview.webrtc.org/2540893003/diff/60001/webrtc/tools/frame_analyz...
> webrtc/tools/frame_analyzer/reference_less_video_analysis_lib.cc:29:
> y4m_input_open(&y4m, input_file, NULL, 0, 0);
> We are using this function since this internally calls "y4m_parse_tags" which
is
> what we need. But since that is not directly exposed in library you are
calling
> this. But is this a good practise? Since this function does lot more than just
> parsing.

If it's possible to only call y4m_parse_tags that's certainly better. I just did
this as a proof-of-concept.

[charujain, 2016-11-30 12:15:08]

On 2016/11/30 10:00:01, kjellander_webrtc wrote:
> On 2016/11/30 09:58:23, charujain wrote:
> > Great. Thanks for creating this. Just one question on this.
> > 
> >
>
https://codereview.webrtc.org/2540893003/diff/60001/webrtc/tools/frame_analyz...
> > File webrtc/tools/frame_analyzer/reference_less_video_analysis_lib.cc
(right):
> > 
> >
>
https://codereview.webrtc.org/2540893003/diff/60001/webrtc/tools/frame_analyz...
> > webrtc/tools/frame_analyzer/reference_less_video_analysis_lib.cc:29:
> > y4m_input_open(&y4m, input_file, NULL, 0, 0);
> > We are using this function since this internally calls "y4m_parse_tags"
which
> is
> > what we need. But since that is not directly exposed in library you are
> calling
> > this. But is this a good practise? Since this function does lot more than
just
> > parsing.
> 
> If it's possible to only call y4m_parse_tags that's certainly better. I just
did
> this as a proof-of-concept.

To call only y4m_parse_tags we will have to ask libvpx team to expose this as an

library which is not something they are going to do soon(mostly). So either I
should
go ahead and use y4m_input_open or be with the current approach which re-write
this code.

[kjellander_webrtc, 2016-11-30 12:21:37]

On 2016/11/30 12:15:08, charujain wrote:
> On 2016/11/30 10:00:01, kjellander_webrtc wrote:
> > On 2016/11/30 09:58:23, charujain wrote:
> > > Great. Thanks for creating this. Just one question on this.
> > > 
> > >
> >
>
https://codereview.webrtc.org/2540893003/diff/60001/webrtc/tools/frame_analyz...
> > > File webrtc/tools/frame_analyzer/reference_less_video_analysis_lib.cc
> (right):
> > > 
> > >
> >
>
https://codereview.webrtc.org/2540893003/diff/60001/webrtc/tools/frame_analyz...
> > > webrtc/tools/frame_analyzer/reference_less_video_analysis_lib.cc:29:
> > > y4m_input_open(&y4m, input_file, NULL, 0, 0);
> > > We are using this function since this internally calls "y4m_parse_tags"
> which
> > is
> > > what we need. But since that is not directly exposed in library you are
> > calling
> > > this. But is this a good practise? Since this function does lot more than
> just
> > > parsing.
> > 
> > If it's possible to only call y4m_parse_tags that's certainly better. I just
> did
> > this as a proof-of-concept.
> 
> To call only y4m_parse_tags we will have to ask libvpx team to expose this as
an
> 
> library which is not something they are going to do soon(mostly). So either I
> should
> go ahead and use y4m_input_open or be with the current approach which re-write
> this code.

I see now that y4m_parse_tags doesn't really make sense, since you need to do
parsing to be able to do that.
I don't see why we wouldn't just use the existing y4m_input_open function. All
it does is filling the y4m_input struct.
Even if we don't use all of that I don't think that's any problem - duplicating
code for parsing a complex format is much worse.

[phoglund, 2016-11-30 12:41:00]

phoglund@webrtc.org changed reviewers:
+ phoglund@webrtc.org

[phoglund, 2016-11-30 12:41:01]

https://codereview.webrtc.org/2540893003/diff/60001/webrtc/tools/frame_analyz...
File webrtc/tools/frame_analyzer/reference_less_video_analysis_lib.cc (right):

https://codereview.webrtc.org/2540893003/diff/60001/webrtc/tools/frame_analyz...
webrtc/tools/frame_analyzer/reference_less_video_analysis_lib.cc:19: #include
"third_party/libvpx/source/libvpx/y4minput.h"
Oh, so you could just include the file. Nice! Sorry Charu, I was wrong about
that. I thought there was a problem including the file for some reason but
apparently not.

[kjellander_webrtc, 2016-12-02 11:27:17]

Description was changed from

==========
Proof of concept using libvpx's y4minput.c

This prevents reinventing the wheel in https://codereview.webrtc.org/2515253004
==========

to

==========
Proof of concept using libvpx's y4minput.c

This prevents reinventing the wheel in https://codereview.webrtc.org/2515253004
==========