| Message ID | 20160803213104.4792-1-mindmark@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On Wed, Aug 03, 2016 at 02:31:04PM -0700, Mark Reid wrote: > setting the codec_tag no longer needed by movenc > it uses the dnxhr profile instead > --- > libavcodec/dnxhdenc.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/libavcodec/dnxhdenc.c b/libavcodec/dnxhdenc.c > index b0ee8a2..cbf4cd5 100644 > --- a/libavcodec/dnxhdenc.c > +++ b/libavcodec/dnxhdenc.c > @@ -341,9 +341,6 @@ static av_cold int dnxhd_encode_init(AVCodecContext *avctx) > } > av_log(avctx, AV_LOG_DEBUG, "cid %d\n", ctx->cid); > > - if (ctx->cid >= 1270 && ctx->cid <= 1274) > - avctx->codec_tag = MKTAG('A','V','d','h'); does this not break older libavformat ? newer libavcodec could be used with older libavformat [...]
On 8/5/16, Michael Niedermayer <michael@niedermayer.cc> wrote: > On Wed, Aug 03, 2016 at 02:31:04PM -0700, Mark Reid wrote: >> setting the codec_tag no longer needed by movenc >> it uses the dnxhr profile instead >> --- >> libavcodec/dnxhdenc.c | 3 --- >> 1 file changed, 3 deletions(-) >> >> diff --git a/libavcodec/dnxhdenc.c b/libavcodec/dnxhdenc.c >> index b0ee8a2..cbf4cd5 100644 >> --- a/libavcodec/dnxhdenc.c >> +++ b/libavcodec/dnxhdenc.c >> @@ -341,9 +341,6 @@ static av_cold int dnxhd_encode_init(AVCodecContext >> *avctx) >> } >> av_log(avctx, AV_LOG_DEBUG, "cid %d\n", ctx->cid); >> >> - if (ctx->cid >= 1270 && ctx->cid <= 1274) >> - avctx->codec_tag = MKTAG('A','V','d','h'); > > does this not break older libavformat ? > newer libavcodec could be used with older libavformat Are you serious? That kind of scenario is bad idea and it never worked and was never tested. > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Asymptotically faster algorithms should always be preferred if you have > asymptotical amounts of data >
On Fri, Aug 05, 2016 at 12:58:10PM +0200, Paul B Mahol wrote: > On 8/5/16, Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Wed, Aug 03, 2016 at 02:31:04PM -0700, Mark Reid wrote: > >> setting the codec_tag no longer needed by movenc > >> it uses the dnxhr profile instead > >> --- > >> libavcodec/dnxhdenc.c | 3 --- > >> 1 file changed, 3 deletions(-) > >> > >> diff --git a/libavcodec/dnxhdenc.c b/libavcodec/dnxhdenc.c > >> index b0ee8a2..cbf4cd5 100644 > >> --- a/libavcodec/dnxhdenc.c > >> +++ b/libavcodec/dnxhdenc.c > >> @@ -341,9 +341,6 @@ static av_cold int dnxhd_encode_init(AVCodecContext > >> *avctx) > >> } > >> av_log(avctx, AV_LOG_DEBUG, "cid %d\n", ctx->cid); > >> > >> - if (ctx->cid >= 1270 && ctx->cid <= 1274) > >> - avctx->codec_tag = MKTAG('A','V','d','h'); > > > > does this not break older libavformat ? > > newer libavcodec could be used with older libavformat > > Are you serious? That kind of scenario is bad idea and it never worked > and was never tested. its tested by the users of binary packages in linux distros debian and ubuntu being examples each lib has major, minor and micro versions MAJOR version when you make incompatible API changes, MINOR version when you add functionality in a backwards-compatible manner, and PATCH version when you make backwards-compatible bug fixes. (taken from http://semver.org/) so if minor is bumped in libavcodec then it has a new feature and libavformat at that point and later might need that feature and thus there will be a matching dependancy libavformat->libavcodec >= x.y but if libavformat bumps minor, libavcodec does not depend on libavcodec thus there wont be a dependancy. any lib with larger minor version must work as a drop in for older versions of same major version you can also see this in the actual dependancies on ubuntu for example: apt-cache show libavformat-ffmpeg56 libavcodec-ffmpeg56 | grep Depends Depends: libavcodec-ffmpeg56 (>= 7:2.7) | libavcodec-ffmpeg-extra56 (>= 7:2.7), libavutil-ffmpeg54 (>= 7:2.8), libbluray1 (>= 1:0.2.2), libbz2-1.0, libc6 (>= 2.14), libgme0 (>= 0.5.5), libgnutls30 (>= 3.4.2), libmodplug1 (>= 1:0.8.8.5), librtmp1 (>= 2.3), libssh-gcrypt-4 (>= 0.4.2), zlib1g (>= 1:1.2.0.2) Depends: libavutil-ffmpeg54 (>= 7:2.6), libc6 (>= 2.14), libcrystalhd3 (>= 1:0.0~git20110715.fdd2f19), libgsm1 (>= 1.0.13), liblzma5 (>= 5.1.1alpha+20120614), libmp3lame0, libopenjpeg5 (>= 1.3+dfsg), libopus0 (>= 1.1), libschroedinger-1.0-0 (>= 1.0.0), libshine3 (>= 3.1.0), libsnappy1v5, libspeex1 (>= 1.2~beta3-1), libswresample-ffmpeg1 (>= 7:2.4), libtheora0 (>= 1.0), libtwolame0, libva1 (>= 1.0.3), libvorbis0a (>= 1.1.2), libvorbisenc2 (>= 1.1.2), libvpx3 (>= 1.5.0), libwavpack1 (>= 4.40.0), libwebp5 (>= 0.4.3), libx264-148, libx265-79 (>= 1.9), libxvidcore4 (>= 1.2.2), libzvbi0 (>= 0.2.35), zlib1g (>= 1:1.2.0) [...]
On Fri, Aug 5, 2016 at 4:28 AM, Michael Niedermayer <michael@niedermayer.cc> wrote: > On Fri, Aug 05, 2016 at 12:58:10PM +0200, Paul B Mahol wrote: >> On 8/5/16, Michael Niedermayer <michael@niedermayer.cc> wrote: >> > On Wed, Aug 03, 2016 at 02:31:04PM -0700, Mark Reid wrote: >> >> setting the codec_tag no longer needed by movenc >> >> it uses the dnxhr profile instead >> >> --- >> >> libavcodec/dnxhdenc.c | 3 --- >> >> 1 file changed, 3 deletions(-) >> >> >> >> diff --git a/libavcodec/dnxhdenc.c b/libavcodec/dnxhdenc.c >> >> index b0ee8a2..cbf4cd5 100644 >> >> --- a/libavcodec/dnxhdenc.c >> >> +++ b/libavcodec/dnxhdenc.c >> >> @@ -341,9 +341,6 @@ static av_cold int dnxhd_encode_init(AVCodecContext >> >> *avctx) >> >> } >> >> av_log(avctx, AV_LOG_DEBUG, "cid %d\n", ctx->cid); >> >> >> >> - if (ctx->cid >= 1270 && ctx->cid <= 1274) >> >> - avctx->codec_tag = MKTAG('A','V','d','h'); >> > >> > does this not break older libavformat ? >> > newer libavcodec could be used with older libavformat >> In that case I can just leave it as is and drop the patch. It was bit unclear as to whether the encoder was allowed to set codec_tag and thats why I was trying to clean it up. >> Are you serious? That kind of scenario is bad idea and it never worked >> and was never tested. > > its tested by the users of binary packages in linux distros > debian and ubuntu being examples > > each lib has major, minor and micro versions > > MAJOR version when you make incompatible API changes, > MINOR version when you add functionality in a backwards-compatible manner, and > PATCH version when you make backwards-compatible bug fixes. > (taken from http://semver.org/) > > so if minor is bumped in libavcodec then it has a new feature and > libavformat at that point and later might need that feature and > thus there will be a matching dependancy libavformat->libavcodec >= x.y > > but if libavformat bumps minor, libavcodec does not depend on > libavcodec thus there wont be a dependancy. > > any lib with larger minor version must work as a drop in for older > versions of same major version > > you can also see this in the actual dependancies on ubuntu for example: > > apt-cache show libavformat-ffmpeg56 libavcodec-ffmpeg56 | grep Depends > > Depends: libavcodec-ffmpeg56 (>= 7:2.7) | libavcodec-ffmpeg-extra56 (>= 7:2.7), libavutil-ffmpeg54 (>= 7:2.8), libbluray1 (>= 1:0.2.2), libbz2-1.0, libc6 (>= 2.14), libgme0 (>= 0.5.5), libgnutls30 (>= 3.4.2), libmodplug1 (>= 1:0.8.8.5), librtmp1 (>= 2.3), libssh-gcrypt-4 (>= 0.4.2), zlib1g (>= 1:1.2.0.2) > > Depends: libavutil-ffmpeg54 (>= 7:2.6), libc6 (>= 2.14), libcrystalhd3 > (>= 1:0.0~git20110715.fdd2f19), libgsm1 (>= 1.0.13), liblzma5 (>= 5.1.1alpha+20120614), libmp3lame0, libopenjpeg5 (>= 1.3+dfsg), libopus0 (>= 1.1), libschroedinger-1.0-0 (>= 1.0.0), libshine3 (>= 3.1.0), libsnappy1v5, libspeex1 (>= 1.2~beta3-1), libswresample-ffmpeg1 (>= 7:2.4), libtheora0 (>= 1.0), libtwolame0, libva1 (>= 1.0.3), libvorbis0a (>= 1.1.2), libvorbisenc2 (>= 1.1.2), libvpx3 (>= 1.5.0), libwavpack1 (>= 4.40.0), libwebp5 (>= 0.4.3), libx264-148, libx265-79 (>= 1.9), libxvidcore4 (>= 1.2.2), libzvbi0 (>= 0.2.35), zlib1g (>= 1:1.2.0) > as this would be a small change in behaviour and not a binary incompatibility would this then be a MINOR bump? > [...] > > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > The educated differ from the uneducated as much as the living from the > dead. -- Aristotle > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On Sun, Aug 07, 2016 at 04:35:35PM -0700, Mark Reid wrote: > On Fri, Aug 5, 2016 at 4:28 AM, Michael Niedermayer > <michael@niedermayer.cc> wrote: > > On Fri, Aug 05, 2016 at 12:58:10PM +0200, Paul B Mahol wrote: > >> On 8/5/16, Michael Niedermayer <michael@niedermayer.cc> wrote: > >> > On Wed, Aug 03, 2016 at 02:31:04PM -0700, Mark Reid wrote: > >> >> setting the codec_tag no longer needed by movenc > >> >> it uses the dnxhr profile instead > >> >> --- > >> >> libavcodec/dnxhdenc.c | 3 --- > >> >> 1 file changed, 3 deletions(-) > >> >> > >> >> diff --git a/libavcodec/dnxhdenc.c b/libavcodec/dnxhdenc.c > >> >> index b0ee8a2..cbf4cd5 100644 > >> >> --- a/libavcodec/dnxhdenc.c > >> >> +++ b/libavcodec/dnxhdenc.c > >> >> @@ -341,9 +341,6 @@ static av_cold int dnxhd_encode_init(AVCodecContext > >> >> *avctx) > >> >> } > >> >> av_log(avctx, AV_LOG_DEBUG, "cid %d\n", ctx->cid); > >> >> > >> >> - if (ctx->cid >= 1270 && ctx->cid <= 1274) > >> >> - avctx->codec_tag = MKTAG('A','V','d','h'); > >> > > >> > does this not break older libavformat ? > >> > newer libavcodec could be used with older libavformat > >> > > In that case I can just leave it as is and drop the patch. It was bit > unclear as to whether the encoder was allowed to set codec_tag and > thats why I was trying to clean it up. you could put it under some #if FF_API_WHATEVER so we get rid of this eventually ... > > >> Are you serious? That kind of scenario is bad idea and it never worked > >> and was never tested. > > > > its tested by the users of binary packages in linux distros > > debian and ubuntu being examples > > > > each lib has major, minor and micro versions > > > > MAJOR version when you make incompatible API changes, > > MINOR version when you add functionality in a backwards-compatible manner, and > > PATCH version when you make backwards-compatible bug fixes. > > (taken from http://semver.org/) [...] > as this would be a small change in behaviour and not a binary > incompatibility would this then be a MINOR bump? IIUC its not backwards-compatible [...]
diff --git a/libavcodec/dnxhdenc.c b/libavcodec/dnxhdenc.c index b0ee8a2..cbf4cd5 100644 --- a/libavcodec/dnxhdenc.c +++ b/libavcodec/dnxhdenc.c @@ -341,9 +341,6 @@ static av_cold int dnxhd_encode_init(AVCodecContext *avctx) } av_log(avctx, AV_LOG_DEBUG, "cid %d\n", ctx->cid); - if (ctx->cid >= 1270 && ctx->cid <= 1274) - avctx->codec_tag = MKTAG('A','V','d','h'); - if (avctx->width < 256 || avctx->height < 120) { av_log(avctx, AV_LOG_ERROR, "Input dimensions too small, input must be at least 256x120\n");