On Sun, Sep 28, 2014 at 3:16 AM, Michael Niedermayer <michae...@gmx.at>
wrote:
> On Sun, Sep 28, 2014 at 11:04:34AM +0200, Tomas Härdin wrote:
> > On Thu, 2014-09-25 at 16:13 -0700, Mark Reid wrote:
> > > ---
> > > libavformat/mxfdec.c | 118
> ++++++++++++++++++++++++++++++++++++++++++---------
> > > 1 file changed, 97 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> > > index 7a4633f..3a1889f 100644
> > > --- a/libavformat/mxfdec.c
> > > +++ b/libavformat/mxfdec.c
> > > @@ -188,6 +188,7 @@ typedef struct {
> > > int tracks_count;
> > > MXFDescriptor *descriptor; /* only one */
> > > UID descriptor_ref;
> > > + char *name;
> > > } MXFPackage;
> > >
> > > typedef struct {
> > > @@ -731,6 +732,27 @@ static int mxf_read_sequence(void *arg,
> AVIOContext *pb, int tag, int size, UID
> > > return 0;
> > > }
> > >
> > > +static int mxf_read_utf16_string(AVIOContext *pb, int size, char**
> str)
> > > +{
> > > + int ret;
> > > + size_t buf_size;
> > > +
> > > + if (size < 0)
> > > + return AVERROR(EINVAL);
> > > +
> > > + buf_size = size + size / 2 + 1;
> >
> > This should be a function, like ff_utf16_buflen() or something. I see
> > that this hunk is just moving the function though, so don't let that
> > hold this patch up.
> >
> > > + *str = av_malloc(buf_size);
> > > + if (!*str)
> > > + return AVERROR(ENOMEM);
> > > +
> > > + if ((ret = avio_get_str16be(pb, size, *str, buf_size)) < 0) {
> > > + av_freep(str);
> > > + return ret;
> > > + }
> > > +
> > > + return ret;
> > > +}
> >
> >
> > > +static int mxf_parse_physical_source_package(MXFContext *mxf,
> MXFTrack *source_track, AVStream *st)
> > > +{
> > > [...]
>
> > > +
> > > + for (k = 0; k <
> physical_track->sequence->structural_components_count; k++) {
> > > + component = mxf_resolve_strong_ref(mxf,
> &physical_track->sequence->structural_components_refs[k],
> TimecodeComponent);
> > > + if (!component)
> > > + continue;
> > > +
> > > + mxf_tc = (MXFTimecodeComponent*)component;
> > > + flags = mxf_tc->drop_frame == 1 ?
> AV_TIMECODE_FLAG_DROPFRAME : 0;
> > > + /* scale sourceclip start_position to match physical
> track edit rate */
> > > + start_position =
> av_rescale_q(sourceclip->start_position, av_inv_q(source_track->edit_rate),
> av_inv_q(physical_track->edit_rate));
> >
> > av_rescale()
>
> edit_rate is a AVRational, so av_rescale_q() seems fitting to me
> or do i misunderstand ?
> the av_inv_q() could be avoided though by exchanging the 2 arguments
>
> [...]
>
yeah, the arguments should simply be reversed, i was just thinking
backwards, I'll fix it and send a new patch. I did a bit of fuzz testing
with zzuf, but I'll do a bit more before submitting again. Thanks for
taking the time to review.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
