On Tue, May 5, 2020 at 5:11 AM Kieran O Leary <kieran.o.le...@gmail.com>
wrote:
> Hi,
>
> I broke the threading with my last reply, i apologise. Here goes another
> attempt:
>
> On Tue, Apr 28, 2020 at 6:23 PM Neil Birkbeck <neil.birkb...@gmail.com>
> wrote:
>
> > On Tue, Apr 28, 2020 at 3:18 AM Nicolas George <geo...@nsup.org> wrote:
> >
> > > Andreas Rheinhardt (12020-04-28):
> > > > That's expected. The patch provided only provides the structure in
> > which
> > > > the values are intended to be exported; it does not add any demuxing
> or
> > > > muxing capabilities for mov or mkv (as you can see from the fact that
> > > > none of these (de)muxers have been changed in the patch).
> > >
> > > Which is something I intended to comment on: adding code without users
> > > is rarely a good idea. I suggest we do not commit until at least one
> > > demuxer use it, preferably at least two. Otherwise, we may realize that
> > > “oh crap, it doesn't work” because of a tiny unforeseen detail.
> >
> >
> > Thanks for the feedback. I also have patches for the demux (MOV/MKV) and
> > mux (MOV/MKV).
> >
> > As there is still the alternative of using the fields in the
> > AVCodecParameters/AVCodecContext, my intention was to keep the first
> patch
> > small to resolve discussion on that point.
> >
> > I've included the patches, if you'd like to try test it, Kieren. I see on
> > your test file that there may be some slight rounding error making output
> > crop 704 not 703 (MKV file ends up with pixel_crop_{left,right} = 8).
> >
> > /ffprobe ../testdata/clap.mov 2>&1 | grep -A1 "Side"
> > Side data:
> > Clean aperture:[width 41472/59 height:576/1 h_offset:0/1
> > v_offset:0/1]
> > ./ffmpeg -i ../testdata/clap.mov -vcodec copy -acodec copy /tmp/clap.mkv
> > ./ffprobe /tmp/clap.mkv 2>&1 | grep -A1 "Side"
> > Side data:
> > Clean aperture:[width 704/1 height:576/1 h_offset:0/1 v_offset:0/1]
> >
>
> I have to look deeper into the MKV side of things and most likely raise it
> with the cellar mailing list so that better minds than mine can weigh in. I
> do see that the rounding up to 704 could be an issue alright.
> As for MOV, your patch appears to generate the same output clap values as
> the input, so that's really great! command line and mediainfo trace below:
>
Thanks for testing, Kieran and for linking the discussion on the cellar
list.
Any additional thoughts from ffmpeg devs on container-level SideData vs
adding the extra fields into AVCodecParameters/AVCodecContext and plumbing
into the frame instead? I anticipate some situations where there can be
interaction between cropping in bitstream and container-level cropping.
Maybe the best way forward is for me to share some sample patches for the
alternative to validate whether it supports the various use cases
(transmux, decode+crop, any other application level handling), and to
confirm the interface changes for the structs.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
