Re: [FFmpeg-devel] [PATCH 2/2] lavfi/af_anequalizer: replace pow(x, -2) by 1/(x*x)
On 12/28/15, Ganesh Ajjanagadde wrote: > Signed-off-by: Ganesh Ajjanagadde > --- > libavfilter/af_anequalizer.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/libavfilter/af_anequalizer.c b/libavfilter/af_anequalizer.c > index d7b5b6c..649c0b9 100644 > --- a/libavfilter/af_anequalizer.c > +++ b/libavfilter/af_anequalizer.c > @@ -391,8 +391,8 @@ static void chebyshev1_bp_filter(EqualizatorFilter *f, > > epsilon = sqrt((G*G - Gb*Gb) / (Gb*Gb - G0*G0)); > g0 = pow(G0,1.0/N); > -alfa = pow(1.0/epsilon+ sqrt(1 + pow(epsilon,-2.0)), 1.0/N); > -beta = pow(G/epsilon + Gb * sqrt(1 + pow(epsilon,-2.0)), 1.0/N); > +alfa = pow(1.0/epsilon+ sqrt(1 + 1/(epsilon*epsilon)), 1.0/N); > +beta = pow(G/epsilon + Gb * sqrt(1 + 1/(epsilon*epsilon)), 1.0/N); > a = 0.5 * (alfa - 1.0/alfa); > b = 0.5 * (beta - g0*g0*(1/beta)); > tetta_b = tan(wb/2); > -- > 2.6.4 > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > both patches ok ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] x86/vf_stereo3d: make ff_anaglyph_sse4 work on x86_32
On 12/27/15, James Almer wrote: > Signed-off-by: James Almer > --- > libavfilter/x86/vf_stereo3d.asm| 47 > +++--- > libavfilter/x86/vf_stereo3d_init.c | 2 +- > 2 files changed, 45 insertions(+), 4 deletions(-) > > diff --git a/libavfilter/x86/vf_stereo3d.asm > b/libavfilter/x86/vf_stereo3d.asm > index 29a8c56..491579f 100644 > --- a/libavfilter/x86/vf_stereo3d.asm > +++ b/libavfilter/x86/vf_stereo3d.asm > @@ -22,8 +22,6 @@ > > %include "libavutil/x86/x86util.asm" > > -%if ARCH_X86_64 > - > SECTION_RODATA > > ; rgbrgbrgbrgb > @@ -37,10 +35,33 @@ ex_b: db 2,-1,-1,-1,5,-1,-1,-1,8,-1,-1,-1,11,-1,-1,-1 > SECTION .text > > INIT_XMM sse4 > +%if ARCH_X86_64 > cglobal anaglyph, 6, 10, 14, 2*6*mmsize, dst, lsrc, rsrc, dst_linesize, > l_linesize, r_linesize, width, height, o, cnt > %define ana_matrix_rq r6q > %define ana_matrix_gq r7q > %define ana_matrix_bq r8q > + > +%else ; ARCH_X86_32 > +%if HAVE_ALIGNED_STACK > +cglobal anaglyph, 3, 7, 8, 2*9*mmsize, dst, lsrc, rsrc, dst_linesize, > l_linesize, o, cnt > +%else > +cglobal anaglyph, 3, 6, 8, 2*9*mmsize, dst, lsrc, rsrc, dst_linesize, o, > cnt > +%define l_linesizeq r4mp > +%endif ; HAVE_ALIGNED_STACK > +%define ana_matrix_rq r3q > +%define ana_matrix_gq r4q > +%define ana_matrix_bq r5q > +%define r_linesizeq r5mp > +%define widthd r6mp > +%define heightd r7mp > +%define m8 [rsp+mmsize*12] > +%define m9 [rsp+mmsize*13] > +%define m10 [rsp+mmsize*14] > +%define m11 [rsp+mmsize*15] > +%define m12 [rsp+mmsize*16] > +%define m13 [rsp+mmsize*17] > +%endif ; ARCH > + > movana_matrix_rq, r8m > movana_matrix_gq, r9m > movana_matrix_bq, r10m > @@ -74,6 +95,7 @@ cglobal anaglyph, 6, 10, 14, 2*6*mmsize, dst, lsrc, rsrc, > dst_linesize, l_linesi > mova [rsp+mmsize*10], m4 > mova [rsp+mmsize*11], m5 > > +%if ARCH_X86_64 > movu m11, [ana_matrix_bq+ 0] > movq m13, [ana_matrix_bq+16] > pshufdm8, m11, q > @@ -84,6 +106,26 @@ cglobal anaglyph, 6, 10, 14, 2*6*mmsize, dst, lsrc, > rsrc, dst_linesize, l_linesi > pshufd m13, m13, q > mov widthd, dword widthm > mov heightd, dword heightm > +%else > +movu m3, [ana_matrix_bq+ 0] > +movq m5, [ana_matrix_bq+16] > +pshufdm0, m3, q > +pshufdm1, m3, q > +pshufdm2, m3, q > +pshufdm3, m3, q > +pshufdm4, m5, q > +pshufdm5, m5, q > +mova [rsp+mmsize*12], m0 > +mova [rsp+mmsize*13], m1 > +mova [rsp+mmsize*14], m2 > +mova [rsp+mmsize*15], m3 > +mova [rsp+mmsize*16], m4 > +mova [rsp+mmsize*17], m5 > +movdst_linesizeq, r3m > +%if HAVE_ALIGNED_STACK > +mov l_linesizeq, r4m > +%endif > +%endif ; ARCH > > .nextrow: > mov od, widthd > @@ -172,4 +214,3 @@ cglobal anaglyph, 6, 10, 14, 2*6*mmsize, dst, lsrc, > rsrc, dst_linesize, l_linesi > sub heightd, 1 > jg .nextrow > REP_RET > -%endif > diff --git a/libavfilter/x86/vf_stereo3d_init.c > b/libavfilter/x86/vf_stereo3d_init.c > index 77d4f7b..da160a8 100644 > --- a/libavfilter/x86/vf_stereo3d_init.c > +++ b/libavfilter/x86/vf_stereo3d_init.c > @@ -31,7 +31,7 @@ void ff_stereo3d_init_x86(Stereo3DDSPContext *dsp) > { > int cpu_flags = av_get_cpu_flags(); > > -if (ARCH_X86_64 && EXTERNAL_SSE4(cpu_flags)) { > +if (EXTERNAL_SSE4(cpu_flags)) { > dsp->anaglyph = ff_anaglyph_sse4; > } > } > -- > 2.6.3 > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > both patches ok if fate is not broken by this ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] ffplay: insertion point of the auto rotation filter - Github ticket #141
On Sun, Dec 27, 2015 at 11:09:42PM +0100, Balint Marton wrote: > Hi Michael, > > The patch you committed seems to break the cropping to even width / > height as required by SDL overlay code. > > Also there can be a use case for inserting the auto rotation filter > after the user provided filter chain as well. (e,g, deinterlacing). > > I don't think we can make everyone happy, so unless you have a > better idea, I think it would be best if you could just revert the > patch. ok, patch reverted [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Dictatorship naturally arises out of democracy, and the most aggravated form of tyranny and slavery out of the most extreme liberty. -- Plato signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] ffplay: insertion point of the auto rotation filter - Github ticket #141
On Sun, Dec 27, 2015 at 11:09:42PM +0100, Balint Marton wrote: > Hi Michael, > > The patch you committed seems to break the cropping to even width / > height as required by SDL overlay code. > > Also there can be a use case for inserting the auto rotation filter > after the user provided filter chain as well. (e,g, deinterlacing). do you have a sample which uses interlacing and rotation ? on second thought this seems a somewhat odd combination rotated videos are something i would associate with phones, interlacing i would generally not associate with phones > > I don't think we can make everyone happy, so unless you have a > better idea, I think it would be best if you could just revert the > patch. > > Thanks, > Marton > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The real ebay dictionary, page 3 "Rare item" - "Common item with rare defect or maybe just a lie" "Professional" - "'Toy' made in china, not functional except as doorstop" "Experts will know" - "The seller hopes you are not an expert" signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/utils: fix check for invalid timebase when decoding subtitles
On Sun, Dec 27, 2015 at 9:29 PM, Andreas Cadhalpun wrote: > On 27.12.2015 21:13, Hendrik Leppkes wrote: >> On Sun, Dec 27, 2015 at 9:03 PM, Andreas Cadhalpun >> wrote: >>> On 27.12.2015 20:43, Hendrik Leppkes wrote: On Sun, Dec 27, 2015 at 8:29 PM, Andreas Cadhalpun wrote: > On 27.12.2015 20:10, Hendrik Leppkes wrote: >> Invalid timebases have a zero numerator, not denominator. > > A timebase with zero numerator is probably invalid, but a timebase > with zero denominator is not even well defined. > So this comment doesn't seem quite right. > >> Fixes a integer divison by zero. >> --- >> libavcodec/utils.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libavcodec/utils.c b/libavcodec/utils.c >> index 19f3f0a..33295ed 100644 >> --- a/libavcodec/utils.c >> +++ b/libavcodec/utils.c >> @@ -2435,7 +2435,7 @@ int avcodec_decode_subtitle2(AVCodecContext >> *avctx, AVSubtitle *sub, >> } else { >> avctx->internal->pkt = &pkt_recoded; >> >> -if (avctx->pkt_timebase.den && avpkt->pts != AV_NOPTS_VALUE) >> +if (avctx->pkt_timebase.num && avpkt->pts != AV_NOPTS_VALUE) >> sub->pts = av_rescale_q(avpkt->pts, >> avctx->pkt_timebase, >> AV_TIME_BASE_Q); >> ret = avctx->codec->decode(avctx, sub, got_sub_ptr, >> &pkt_recoded); >> > > If avctx->pkt_timebase.den is 0, calling av_rescale_q here will trigger > the > av_assert2(c > 0) in av_rescale_rnd, so removing this check isn't good. > > Why not check for both num and den? > We never check both, invalid timebase is {0, 1}, anything else needs to have values in both. >>> >>> I'd call this unknown timebase, as {0, 1} is the initialization value. >>> A {1, 0} timebase is certainly not valid. >>> All timebase checks are for num only. >>> >>> The check here was for den and there is a similar check in >>> teletext_decode_frame. >> >> And thats a bug since that can actually lead to div by 0 and crash. > > Then please fix this bug also in teletext_decode_frame. > Pushed with the second fix added and slightly reworded message. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On Mon, Dec 28, 2015 at 03:10:18AM +, Carl Eugen Hoyos wrote: > Michael Niedermayer niedermayer.cc> writes: > > > Patch splited in move and matroska part > > i removed this memcpy() for now from what i > > commited as there is clearly no consenus on it > > I would really have appreciated a real review: > Apart from the unrelated audio fix what audio fix ? > a significant > part of the patch was written by me. If thats the case then the commit message of the patch should have contained credits or it should have added credits to the file header what significant part of your code was used ? Should i revert the patches ? do you have another suggestion ? [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Many things microsoft did are stupid, but not doing something just because microsoft did it is even more stupid. If everything ms did were stupid they would be bankrupt already. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avformat/http: Documented http_proxy option
--- doc/protocols.texi | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/protocols.texi b/doc/protocols.texi index 7c6b9d4..8b2e310 100644 --- a/doc/protocols.texi +++ b/doc/protocols.texi @@ -240,6 +240,9 @@ If set to 1 use chunked Transfer-Encoding for posts, default is 1. @item content_type Set a specific content type for the POST messages. +@item http_proxy +set HTTP proxy to tunnel through e.g. http://example.com:1234 + @item headers Set custom HTTP headers, can override built in default headers. The value must be a string encoding the headers. -- 1.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On 12/28/2015 10:34 AM, Michael Niedermayer wrote: On Mon, Dec 28, 2015 at 03:10:18AM +, Carl Eugen Hoyos wrote: Michael Niedermayer niedermayer.cc> writes: Patch splited in move and matroska part i removed this memcpy() for now from what i commited as there is clearly no consenus on it I would really have appreciated a real review: Apart from the unrelated audio fix what audio fix ? a significant part of the patch was written by me. If thats the case then the commit message of the patch should have contained credits or it should have added credits to the file header what significant part of your code was used ? Should i revert the patches ? do you have another suggestion ? [...] Michael, he's talking about the OLD patch that was never applied. My patch has been written from scratch, more or less. I did borrowed some palette loops from mov.c, but I have also attributed the previous authors at the top of qtpalette.c properly. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On 12/28/2015 10:34 AM, Michael Niedermayer wrote: On Mon, Dec 28, 2015 at 03:10:18AM +, Carl Eugen Hoyos wrote: Michael Niedermayer niedermayer.cc> writes: Patch splited in move and matroska part i removed this memcpy() for now from what i commited as there is clearly no consenus on it I would really have appreciated a real review: Apart from the unrelated audio fix what audio fix ? He probably means the change from 86 to 36 as the lower limit for private data in A_QUICKTIME (36 is the size of a version 0 sound sample description). Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On 12/28/2015 11:46 AM, Mats Peterson wrote: On 12/28/2015 10:34 AM, Michael Niedermayer wrote: On Mon, Dec 28, 2015 at 03:10:18AM +, Carl Eugen Hoyos wrote: Michael Niedermayer niedermayer.cc> writes: Patch splited in move and matroska part i removed this memcpy() for now from what i commited as there is clearly no consenus on it I would really have appreciated a real review: Apart from the unrelated audio fix what audio fix ? He probably means the change from 86 to 36 as the lower limit for private data in A_QUICKTIME (36 is the size of a version 0 sound sample description). Mats We have been talking about that before, Michael. Perhaps I should have emphasized that I've changed that one as well. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
L'octidi 8 nivôse, an CCXXIV, Mats Peterson a écrit : > Michael, he's talking about the OLD patch that was never applied. My patch > has been written from scratch, more or less. I did borrowed some palette > loops from mov.c, but I have also attributed the previous authors at the top > of qtpalette.c properly. I must say, I find this hunk from 7973603: +if (matroska->has_palette) { +uint8_t *pal = av_packet_new_side_data(pkt, AV_PKT_DATA_PALETTE, AVPALETTE_SIZE); +if (!pal) { +av_log(matroska->ctx, AV_LOG_ERROR, "Cannot append palette to packet\n"); +} else { +memcpy(pal, matroska->palette, AVPALETTE_SIZE); +} +matroska->has_palette = 0; +} looks quite similar to this hunk from https://trac.ffmpeg.org/attachment/ticket/5071/patchmkvmov.diff : +if (matroska->pal) { +uint8_t *pal = av_packet_new_side_data(pkt, AV_PKT_DATA_PALETTE, AVPALETTE_SIZE); +if (!pal) { +av_log(matroska->ctx, AV_LOG_ERROR, "Cannot append palette to packet\n"); +} else { +memcpy(pal, matroska->pal, AVPALETTE_SIZE); +} +av_freep(&matroska->pal); +} Especially the use of if/else for error, and actually ignoring the error, instead of the most common (and more correct, but the rest of the code ignores error too) "if...return AVERROR(ENOMEM)". Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On 12/28/2015 12:07 PM, Nicolas George wrote: L'octidi 8 nivôse, an CCXXIV, Mats Peterson a écrit : Michael, he's talking about the OLD patch that was never applied. My patch has been written from scratch, more or less. I did borrowed some palette loops from mov.c, but I have also attributed the previous authors at the top of qtpalette.c properly. I must say, I find this hunk from 7973603: +if (matroska->has_palette) { +uint8_t *pal = av_packet_new_side_data(pkt, AV_PKT_DATA_PALETTE, AVPALETTE_SIZE); +if (!pal) { +av_log(matroska->ctx, AV_LOG_ERROR, "Cannot append palette to packet\n"); +} else { +memcpy(pal, matroska->palette, AVPALETTE_SIZE); +} +matroska->has_palette = 0; +} looks quite similar to this hunk from https://trac.ffmpeg.org/attachment/ticket/5071/patchmkvmov.diff : +if (matroska->pal) { +uint8_t *pal = av_packet_new_side_data(pkt, AV_PKT_DATA_PALETTE, AVPALETTE_SIZE); +if (!pal) { +av_log(matroska->ctx, AV_LOG_ERROR, "Cannot append palette to packet\n"); +} else { +memcpy(pal, matroska->pal, AVPALETTE_SIZE); +} +av_freep(&matroska->pal); +} Especially the use of if/else for error, and actually ignoring the error, instead of the most common (and more correct, but the rest of the code ignores error too) "if...return AVERROR(ENOMEM)". Regards, ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Yes it does, and that's why I said "more or less". That little chunk is mimicked from mov.c for the record, so it's not his invention really. -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On 12/28/2015 12:21 PM, Mats Peterson wrote: On 12/28/2015 12:07 PM, Nicolas George wrote: L'octidi 8 nivôse, an CCXXIV, Mats Peterson a écrit : Michael, he's talking about the OLD patch that was never applied. My patch has been written from scratch, more or less. I did borrowed some palette loops from mov.c, but I have also attributed the previous authors at the top of qtpalette.c properly. I must say, I find this hunk from 7973603: +if (matroska->has_palette) { +uint8_t *pal = av_packet_new_side_data(pkt, AV_PKT_DATA_PALETTE, AVPALETTE_SIZE); +if (!pal) { +av_log(matroska->ctx, AV_LOG_ERROR, "Cannot append palette to packet\n"); +} else { +memcpy(pal, matroska->palette, AVPALETTE_SIZE); +} +matroska->has_palette = 0; +} looks quite similar to this hunk from https://trac.ffmpeg.org/attachment/ticket/5071/patchmkvmov.diff : +if (matroska->pal) { +uint8_t *pal = av_packet_new_side_data(pkt, AV_PKT_DATA_PALETTE, AVPALETTE_SIZE); +if (!pal) { +av_log(matroska->ctx, AV_LOG_ERROR, "Cannot append palette to packet\n"); +} else { +memcpy(pal, matroska->pal, AVPALETTE_SIZE); +} +av_freep(&matroska->pal); +} Especially the use of if/else for error, and actually ignoring the error, instead of the most common (and more correct, but the rest of the code ignores error too) "if...return AVERROR(ENOMEM)". Regards, ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Yes it does, and that's why I said "more or less". That little chunk is mimicked from mov.c for the record, so it's not his invention really. The rest of what you said is hair-splitting in my book. -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On 12/28/2015 12:25 PM, Mats Peterson wrote: On 12/28/2015 12:21 PM, Mats Peterson wrote: On 12/28/2015 12:07 PM, Nicolas George wrote: L'octidi 8 nivôse, an CCXXIV, Mats Peterson a écrit : Michael, he's talking about the OLD patch that was never applied. My patch has been written from scratch, more or less. I did borrowed some palette loops from mov.c, but I have also attributed the previous authors at the top of qtpalette.c properly. I must say, I find this hunk from 7973603: +if (matroska->has_palette) { +uint8_t *pal = av_packet_new_side_data(pkt, AV_PKT_DATA_PALETTE, AVPALETTE_SIZE); +if (!pal) { +av_log(matroska->ctx, AV_LOG_ERROR, "Cannot append palette to packet\n"); +} else { +memcpy(pal, matroska->palette, AVPALETTE_SIZE); +} +matroska->has_palette = 0; +} looks quite similar to this hunk from https://trac.ffmpeg.org/attachment/ticket/5071/patchmkvmov.diff : +if (matroska->pal) { +uint8_t *pal = av_packet_new_side_data(pkt, AV_PKT_DATA_PALETTE, AVPALETTE_SIZE); +if (!pal) { +av_log(matroska->ctx, AV_LOG_ERROR, "Cannot append palette to packet\n"); +} else { +memcpy(pal, matroska->pal, AVPALETTE_SIZE); +} +av_freep(&matroska->pal); +} Especially the use of if/else for error, and actually ignoring the error, instead of the most common (and more correct, but the rest of the code ignores error too) "if...return AVERROR(ENOMEM)". Regards, ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Yes it does, and that's why I said "more or less". That little chunk is mimicked from mov.c for the record, so it's not his invention really. The rest of what you said is hair-splitting in my book. The important thing is not who wrote what (are you listening, Carl?) in the hunt for programming superstardom, but to provide a working solution. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On Mon, Dec 28, 2015 at 11:46:21AM +0100, Mats Peterson wrote: > On 12/28/2015 10:34 AM, Michael Niedermayer wrote: > >On Mon, Dec 28, 2015 at 03:10:18AM +, Carl Eugen Hoyos wrote: > >>Michael Niedermayer niedermayer.cc> writes: > >> > >>>Patch splited in move and matroska part > >>>i removed this memcpy() for now from what i > >>>commited as there is clearly no consenus on it > >> > >>I would really have appreciated a real review: > >>Apart from the unrelated audio fix > > > >what audio fix ? > > > > He probably means the change from 86 to 36 as the lower limit for > private data in A_QUICKTIME (36 is the size of a version 0 sound > sample description). right, i missed that that one should have been a seperate patch [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Concerning the gods, I have no means of knowing whether they exist or not or of what sort they may be, because of the obscurity of the subject, and the brevity of human life -- Protagoras signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On 12/28/2015 02:03 PM, Michael Niedermayer wrote: On Mon, Dec 28, 2015 at 11:46:21AM +0100, Mats Peterson wrote: On 12/28/2015 10:34 AM, Michael Niedermayer wrote: On Mon, Dec 28, 2015 at 03:10:18AM +, Carl Eugen Hoyos wrote: Michael Niedermayer niedermayer.cc> writes: Patch splited in move and matroska part i removed this memcpy() for now from what i commited as there is clearly no consenus on it I would really have appreciated a real review: Apart from the unrelated audio fix what audio fix ? He probably means the change from 86 to 36 as the lower limit for private data in A_QUICKTIME (36 is the size of a version 0 sound sample description). right, i missed that that one should have been a seperate patch [...] ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Well, the proper value, as I said, is 36 rather than 86. Should we redo that part, or is it OK to keep it in the patchset? Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On 12/28/2015 02:09 PM, Mats Peterson wrote: On 12/28/2015 02:03 PM, Michael Niedermayer wrote: On Mon, Dec 28, 2015 at 11:46:21AM +0100, Mats Peterson wrote: On 12/28/2015 10:34 AM, Michael Niedermayer wrote: On Mon, Dec 28, 2015 at 03:10:18AM +, Carl Eugen Hoyos wrote: Michael Niedermayer niedermayer.cc> writes: Patch splited in move and matroska part i removed this memcpy() for now from what i commited as there is clearly no consenus on it I would really have appreciated a real review: Apart from the unrelated audio fix what audio fix ? He probably means the change from 86 to 36 as the lower limit for private data in A_QUICKTIME (36 is the size of a version 0 sound sample description). right, i missed that that one should have been a seperate patch [...] ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Well, the proper value, as I said, is 36 rather than 86. Should we redo that part, or is it OK to keep it in the patchset? Mats Or could you perhaps extract and apply that change on your side, or should I create a single patch for that one on my side? Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On Mon, Dec 28, 2015 at 02:09:54PM +0100, Mats Peterson wrote: > On 12/28/2015 02:03 PM, Michael Niedermayer wrote: > >On Mon, Dec 28, 2015 at 11:46:21AM +0100, Mats Peterson wrote: > >>On 12/28/2015 10:34 AM, Michael Niedermayer wrote: > >>>On Mon, Dec 28, 2015 at 03:10:18AM +, Carl Eugen Hoyos wrote: > Michael Niedermayer niedermayer.cc> writes: > > >Patch splited in move and matroska part > >i removed this memcpy() for now from what i > >commited as there is clearly no consenus on it > > I would really have appreciated a real review: > Apart from the unrelated audio fix > >>> > >>>what audio fix ? > >>> > >> > >>He probably means the change from 86 to 36 as the lower limit for > >>private data in A_QUICKTIME (36 is the size of a version 0 sound > >>sample description). > > > >right, i missed that > >that one should have been a seperate patch > > > >[...] > > > > > > > >___ > >ffmpeg-devel mailing list > >ffmpeg-devel@ffmpeg.org > >http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > Well, the proper value, as I said, is 36 rather than 86. Should we > redo that part, or is it OK to keep it in the patchset? its already in git master so it cannot be truly redone we could revert the single line and recommit it with a more explanatory commit message, this wouldnt remove the previous commit but would add 2 more commits. i suggest we wait for carls oppinion on this, as he was the one complaining about this. [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB DNS cache poisoning attacks, popular search engine, Google internet authority dont be evil, please signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On 12/28/2015 02:16 PM, Michael Niedermayer wrote: On Mon, Dec 28, 2015 at 02:09:54PM +0100, Mats Peterson wrote: On 12/28/2015 02:03 PM, Michael Niedermayer wrote: On Mon, Dec 28, 2015 at 11:46:21AM +0100, Mats Peterson wrote: On 12/28/2015 10:34 AM, Michael Niedermayer wrote: On Mon, Dec 28, 2015 at 03:10:18AM +, Carl Eugen Hoyos wrote: Michael Niedermayer niedermayer.cc> writes: Patch splited in move and matroska part i removed this memcpy() for now from what i commited as there is clearly no consenus on it I would really have appreciated a real review: Apart from the unrelated audio fix what audio fix ? He probably means the change from 86 to 36 as the lower limit for private data in A_QUICKTIME (36 is the size of a version 0 sound sample description). right, i missed that that one should have been a seperate patch [...] ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Well, the proper value, as I said, is 36 rather than 86. Should we redo that part, or is it OK to keep it in the patchset? its already in git master so it cannot be truly redone we could revert the single line and recommit it with a more explanatory commit message, this wouldnt remove the previous commit but would add 2 more commits. i suggest we wait for carls oppinion on this, as he was the one complaining about this. I thought you had the last word in this? He's the bug tracker maintainer, as far as I know. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On 12/28/2015 02:18 PM, Mats Peterson wrote: On 12/28/2015 02:16 PM, Michael Niedermayer wrote: On Mon, Dec 28, 2015 at 02:09:54PM +0100, Mats Peterson wrote: On 12/28/2015 02:03 PM, Michael Niedermayer wrote: On Mon, Dec 28, 2015 at 11:46:21AM +0100, Mats Peterson wrote: On 12/28/2015 10:34 AM, Michael Niedermayer wrote: On Mon, Dec 28, 2015 at 03:10:18AM +, Carl Eugen Hoyos wrote: Michael Niedermayer niedermayer.cc> writes: Patch splited in move and matroska part i removed this memcpy() for now from what i commited as there is clearly no consenus on it I would really have appreciated a real review: Apart from the unrelated audio fix what audio fix ? He probably means the change from 86 to 36 as the lower limit for private data in A_QUICKTIME (36 is the size of a version 0 sound sample description). right, i missed that that one should have been a seperate patch [...] ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Well, the proper value, as I said, is 36 rather than 86. Should we redo that part, or is it OK to keep it in the patchset? its already in git master so it cannot be truly redone we could revert the single line and recommit it with a more explanatory commit message, this wouldnt remove the previous commit but would add 2 more commits. i suggest we wait for carls oppinion on this, as he was the one complaining about this. I thought you had the last word in this? He's the bug tracker maintainer, as far as I know. Mats And he wasn't really complaining at that audio part, but he erroneously stated that he had written "a significant part" of the patch. Most of it is written from scratch by me, once again. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On Mon, Dec 28, 2015 at 02:18:14PM +0100, Mats Peterson wrote: > On 12/28/2015 02:16 PM, Michael Niedermayer wrote: > >On Mon, Dec 28, 2015 at 02:09:54PM +0100, Mats Peterson wrote: > >>On 12/28/2015 02:03 PM, Michael Niedermayer wrote: > >>>On Mon, Dec 28, 2015 at 11:46:21AM +0100, Mats Peterson wrote: > On 12/28/2015 10:34 AM, Michael Niedermayer wrote: > >On Mon, Dec 28, 2015 at 03:10:18AM +, Carl Eugen Hoyos wrote: > >>Michael Niedermayer niedermayer.cc> writes: > >> > >>>Patch splited in move and matroska part > >>>i removed this memcpy() for now from what i > >>>commited as there is clearly no consenus on it > >> > >>I would really have appreciated a real review: > >>Apart from the unrelated audio fix > > > >what audio fix ? > > > > He probably means the change from 86 to 36 as the lower limit for > private data in A_QUICKTIME (36 is the size of a version 0 sound > sample description). > >>> > >>>right, i missed that > >>>that one should have been a seperate patch > >>> > >>>[...] > >>> > >>> > >>> > >>>___ > >>>ffmpeg-devel mailing list > >>>ffmpeg-devel@ffmpeg.org > >>>http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >>> > >> > >>Well, the proper value, as I said, is 36 rather than 86. Should we > >>redo that part, or is it OK to keep it in the patchset? > > > >its already in git master so it cannot be truly redone > >we could revert the single line and recommit it with a more explanatory > >commit message, this wouldnt remove the previous commit but would add 2 > >more commits. > >i suggest we wait for carls oppinion on this, as he was the one > >complaining about this. > > > > I thought you had the last word in this? He's the bug tracker > maintainer, as far as I know. He too is a deveoper like I am and you are. Lets try to find a solution that everyone is happy with Iam happy with anything you and carl agree about [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB In a rich man's house there is no place to spit but his face. -- Diogenes of Sinope signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/matroskadec.c: Copy QuickTime palette to extradata
On Mon, 28 Dec 2015 05:18:41 +0100 Mats Peterson wrote: > I still insist on copying the QuickTime palette to extradata in > matroskadec.c, since it's currently needed for MPlayer to use the > correct palette. As I have explained so many times before, MPlayer, for > some inexplicable reason, currently adds *another* palette side data > packet *after* the one added in matroskadec.c, using whatever is in > extradata as the palette. > > Mats > How about you change MPlayer to use the API correctly? (If the problem is apparently that it does not do so.) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On 12/28/2015 02:39 PM, Michael Niedermayer wrote: 1AM +0100, Mats Peterson wrote: I'm afraid he won't agree on ANYTHING I say, after my initial treatment of him on the bug tracker. I have excused myself for that as well, but he's slow to forget... Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
L'octidi 8 nivôse, an CCXXIV, Mats Peterson a écrit : > I'm afraid he won't agree on ANYTHING I say, after my initial treatment of > him on the bug tracker. I have excused myself for that as well, but he's > slow to forget... Maybe if you stopped ADDING insults like that, it would start making things better. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On 12/28/2015 02:39 PM, Michael Niedermayer wrote: On Mon, Dec 28, 2015 at 02:18:14PM +0100, Mats Peterson wrote: On 12/28/2015 02:16 PM, Michael Niedermayer wrote: On Mon, Dec 28, 2015 at 02:09:54PM +0100, Mats Peterson wrote: On 12/28/2015 02:03 PM, Michael Niedermayer wrote: On Mon, Dec 28, 2015 at 11:46:21AM +0100, Mats Peterson wrote: On 12/28/2015 10:34 AM, Michael Niedermayer wrote: On Mon, Dec 28, 2015 at 03:10:18AM +, Carl Eugen Hoyos wrote: Michael Niedermayer niedermayer.cc> writes: Patch splited in move and matroska part i removed this memcpy() for now from what i commited as there is clearly no consenus on it I would really have appreciated a real review: Apart from the unrelated audio fix what audio fix ? He probably means the change from 86 to 36 as the lower limit for private data in A_QUICKTIME (36 is the size of a version 0 sound sample description). right, i missed that that one should have been a seperate patch [...] ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Well, the proper value, as I said, is 36 rather than 86. Should we redo that part, or is it OK to keep it in the patchset? its already in git master so it cannot be truly redone we could revert the single line and recommit it with a more explanatory commit message, this wouldnt remove the previous commit but would add 2 more commits. i suggest we wait for carls oppinion on this, as he was the one complaining about this. I thought you had the last word in this? He's the bug tracker maintainer, as far as I know. He too is a deveoper like I am and you are. Lets try to find a solution that everyone is happy with Iam happy with anything you and carl agree about I'm afraid he won't agree on ANYTHING I say, after my initial treatment of him on the bug tracker, which I have excused myself for. But he seems slow to forget... Mats ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On 12/28/2015 02:45 PM, Nicolas George wrote: L'octidi 8 nivôse, an CCXXIV, Mats Peterson a écrit : I'm afraid he won't agree on ANYTHING I say, after my initial treatment of him on the bug tracker. I have excused myself for that as well, but he's slow to forget... Maybe if you stopped ADDING insults like that, it would start making things better. Point taken. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On Mon, Dec 28, 2015 at 12:07:38PM +0100, Nicolas George wrote: > L'octidi 8 nivôse, an CCXXIV, Mats Peterson a écrit : > > Michael, he's talking about the OLD patch that was never applied. My patch > > has been written from scratch, more or less. I did borrowed some palette > > loops from mov.c, but I have also attributed the previous authors at the top > > of qtpalette.c properly. > > I must say, I find this hunk from 7973603: > > +if (matroska->has_palette) { > +uint8_t *pal = av_packet_new_side_data(pkt, AV_PKT_DATA_PALETTE, > AVPALETTE_SIZE); > +if (!pal) { > +av_log(matroska->ctx, AV_LOG_ERROR, "Cannot append palette > to packet\n"); > +} else { > +memcpy(pal, matroska->palette, AVPALETTE_SIZE); > +} > +matroska->has_palette = 0; > +} > > looks quite similar to this hunk from > https://trac.ffmpeg.org/attachment/ticket/5071/patchmkvmov.diff : > > +if (matroska->pal) { > +uint8_t *pal = av_packet_new_side_data(pkt, AV_PKT_DATA_PALETTE, > AVPALETTE_SIZE); > +if (!pal) { > +av_log(matroska->ctx, AV_LOG_ERROR, "Cannot append palette > to packet\n"); > +} else { > +memcpy(pal, matroska->pal, AVPALETTE_SIZE); > +} > +av_freep(&matroska->pal); > +} > > Especially the use of if/else for error, and actually ignoring the error, > instead of the most common (and more correct, but the rest of the code > ignores error too) "if...return AVERROR(ENOMEM)". for reference: (similar code prior to the patches) HEAD~20:libavdevice/x11grab.c-pkt->pts = curtime; HEAD~20:libavdevice/x11grab.c-if (s->palette_changed) { HEAD~20:libavdevice/x11grab.c-uint8_t *pal = av_packet_new_side_data(pkt, AV_PKT_DATA_PALETTE, HEAD~20:libavdevice/x11grab.c- AVPALETTE_SIZE); HEAD~20:libavdevice/x11grab.c-if (!pal) { HEAD~20:libavdevice/x11grab.c:av_log(s, AV_LOG_ERROR, "Cannot append palette to packet\n"); HEAD~20:libavdevice/x11grab.c-} else { HEAD~20:libavdevice/x11grab.c-memcpy(pal, s->palette, AVPALETTE_SIZE); HEAD~20:libavdevice/x11grab.c-s->palette_changed = 0; HEAD~20:libavdevice/x11grab.c-} HEAD~20:libavdevice/x11grab.c-} -- HEAD~20:libavformat/asfdec_f.c-if (asf_st->pkt.data && asf_st->palette_changed) { HEAD~20:libavformat/asfdec_f.c-uint8_t *pal; HEAD~20:libavformat/asfdec_f.c-pal = av_packet_new_side_data(&asf_st->pkt, AV_PKT_DATA_PALETTE, HEAD~20:libavformat/asfdec_f.c- AVPALETTE_SIZE); HEAD~20:libavformat/asfdec_f.c-if (!pal) { HEAD~20:libavformat/asfdec_f.c:av_log(s, AV_LOG_ERROR, "Cannot append palette to packet\n"); HEAD~20:libavformat/asfdec_f.c-} else { HEAD~20:libavformat/asfdec_f.c-memcpy(pal, asf_st->palette, AVPALETTE_SIZE); HEAD~20:libavformat/asfdec_f.c-asf_st->palette_changed = 0; HEAD~20:libavformat/asfdec_f.c-} HEAD~20:libavformat/asfdec_f.c-} -- HEAD~20:libavformat/mov.c-if (sc->has_palette) { HEAD~20:libavformat/mov.c-uint8_t *pal; HEAD~20:libavformat/mov.c- HEAD~20:libavformat/mov.c-pal = av_packet_new_side_data(pkt, AV_PKT_DATA_PALETTE, AVPALETTE_SIZE); HEAD~20:libavformat/mov.c-if (!pal) { HEAD~20:libavformat/mov.c:av_log(mov->fc, AV_LOG_ERROR, "Cannot append palette to packet\n"); HEAD~20:libavformat/mov.c-} else { HEAD~20:libavformat/mov.c-memcpy(pal, sc->palette, AVPALETTE_SIZE); HEAD~20:libavformat/mov.c-sc->has_palette = 0; HEAD~20:libavformat/mov.c-} HEAD~20:libavformat/mov.c-} [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Dictatorship naturally arises out of democracy, and the most aggravated form of tyranny and slavery out of the most extreme liberty. -- Plato signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On 12/28/2015 03:29 PM, Michael Niedermayer wrote: On Mon, Dec 28, 2015 at 12:07:38PM +0100, Nicolas George wrote: L'octidi 8 nivôse, an CCXXIV, Mats Peterson a écrit : Michael, he's talking about the OLD patch that was never applied. My patch has been written from scratch, more or less. I did borrowed some palette loops from mov.c, but I have also attributed the previous authors at the top of qtpalette.c properly. I must say, I find this hunk from 7973603: +if (matroska->has_palette) { +uint8_t *pal = av_packet_new_side_data(pkt, AV_PKT_DATA_PALETTE, AVPALETTE_SIZE); +if (!pal) { +av_log(matroska->ctx, AV_LOG_ERROR, "Cannot append palette to packet\n"); +} else { +memcpy(pal, matroska->palette, AVPALETTE_SIZE); +} +matroska->has_palette = 0; +} looks quite similar to this hunk from https://trac.ffmpeg.org/attachment/ticket/5071/patchmkvmov.diff : +if (matroska->pal) { +uint8_t *pal = av_packet_new_side_data(pkt, AV_PKT_DATA_PALETTE, AVPALETTE_SIZE); +if (!pal) { +av_log(matroska->ctx, AV_LOG_ERROR, "Cannot append palette to packet\n"); +} else { +memcpy(pal, matroska->pal, AVPALETTE_SIZE); +} +av_freep(&matroska->pal); +} Especially the use of if/else for error, and actually ignoring the error, instead of the most common (and more correct, but the rest of the code ignores error too) "if...return AVERROR(ENOMEM)". for reference: (similar code prior to the patches) HEAD~20:libavdevice/x11grab.c-pkt->pts = curtime; HEAD~20:libavdevice/x11grab.c-if (s->palette_changed) { HEAD~20:libavdevice/x11grab.c-uint8_t *pal = av_packet_new_side_data(pkt, AV_PKT_DATA_PALETTE, HEAD~20:libavdevice/x11grab.c- AVPALETTE_SIZE); HEAD~20:libavdevice/x11grab.c-if (!pal) { HEAD~20:libavdevice/x11grab.c:av_log(s, AV_LOG_ERROR, "Cannot append palette to packet\n"); HEAD~20:libavdevice/x11grab.c-} else { HEAD~20:libavdevice/x11grab.c-memcpy(pal, s->palette, AVPALETTE_SIZE); HEAD~20:libavdevice/x11grab.c-s->palette_changed = 0; HEAD~20:libavdevice/x11grab.c-} HEAD~20:libavdevice/x11grab.c-} -- HEAD~20:libavformat/asfdec_f.c-if (asf_st->pkt.data && asf_st->palette_changed) { HEAD~20:libavformat/asfdec_f.c-uint8_t *pal; HEAD~20:libavformat/asfdec_f.c-pal = av_packet_new_side_data(&asf_st->pkt, AV_PKT_DATA_PALETTE, HEAD~20:libavformat/asfdec_f.c- AVPALETTE_SIZE); HEAD~20:libavformat/asfdec_f.c-if (!pal) { HEAD~20:libavformat/asfdec_f.c:av_log(s, AV_LOG_ERROR, "Cannot append palette to packet\n"); HEAD~20:libavformat/asfdec_f.c-} else { HEAD~20:libavformat/asfdec_f.c-memcpy(pal, asf_st->palette, AVPALETTE_SIZE); HEAD~20:libavformat/asfdec_f.c-asf_st->palette_changed = 0; HEAD~20:libavformat/asfdec_f.c-} HEAD~20:libavformat/asfdec_f.c-} -- HEAD~20:libavformat/mov.c-if (sc->has_palette) { HEAD~20:libavformat/mov.c-uint8_t *pal; HEAD~20:libavformat/mov.c- HEAD~20:libavformat/mov.c-pal = av_packet_new_side_data(pkt, AV_PKT_DATA_PALETTE, AVPALETTE_SIZE); HEAD~20:libavformat/mov.c-if (!pal) { HEAD~20:libavformat/mov.c:av_log(mov->fc, AV_LOG_ERROR, "Cannot append palette to packet\n"); HEAD~20:libavformat/mov.c-} else { HEAD~20:libavformat/mov.c-memcpy(pal, sc->palette, AVPALETTE_SIZE); HEAD~20:libavformat/mov.c-sc->has_palette = 0; HEAD~20:libavformat/mov.c-} HEAD~20:libavformat/mov.c-} [...] ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Interesting ;) -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
L'octidi 8 nivôse, an CCXXIV, Michael Niedermayer a écrit : > for reference: (similar code prior to the patches) Indeed. Mats already pointed one of them. This would warrant some refactoring. At the very least: ff_frame_new_palette(frame, palette); Then Ganesh will be able to add av_warn_unused_result on it ;) (And in the meantime, it could set AV_FRAME_FLAG_CORRUPT.= Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On 12/28/2015 03:32 PM, Mats Peterson wrote: On 12/28/2015 03:29 PM, Michael Niedermayer wrote: On Mon, Dec 28, 2015 at 12:07:38PM +0100, Nicolas George wrote: L'octidi 8 nivôse, an CCXXIV, Mats Peterson a écrit : Michael, he's talking about the OLD patch that was never applied. My patch has been written from scratch, more or less. I did borrowed some palette loops from mov.c, but I have also attributed the previous authors at the top of qtpalette.c properly. I must say, I find this hunk from 7973603: +if (matroska->has_palette) { +uint8_t *pal = av_packet_new_side_data(pkt, AV_PKT_DATA_PALETTE, AVPALETTE_SIZE); +if (!pal) { +av_log(matroska->ctx, AV_LOG_ERROR, "Cannot append palette to packet\n"); +} else { +memcpy(pal, matroska->palette, AVPALETTE_SIZE); +} +matroska->has_palette = 0; +} looks quite similar to this hunk from https://trac.ffmpeg.org/attachment/ticket/5071/patchmkvmov.diff : +if (matroska->pal) { +uint8_t *pal = av_packet_new_side_data(pkt, AV_PKT_DATA_PALETTE, AVPALETTE_SIZE); +if (!pal) { +av_log(matroska->ctx, AV_LOG_ERROR, "Cannot append palette to packet\n"); +} else { +memcpy(pal, matroska->pal, AVPALETTE_SIZE); +} +av_freep(&matroska->pal); +} Especially the use of if/else for error, and actually ignoring the error, instead of the most common (and more correct, but the rest of the code ignores error too) "if...return AVERROR(ENOMEM)". for reference: (similar code prior to the patches) HEAD~20:libavdevice/x11grab.c-pkt->pts = curtime; HEAD~20:libavdevice/x11grab.c-if (s->palette_changed) { HEAD~20:libavdevice/x11grab.c-uint8_t *pal = av_packet_new_side_data(pkt, AV_PKT_DATA_PALETTE, HEAD~20:libavdevice/x11grab.c- AVPALETTE_SIZE); HEAD~20:libavdevice/x11grab.c-if (!pal) { HEAD~20:libavdevice/x11grab.c:av_log(s, AV_LOG_ERROR, "Cannot append palette to packet\n"); HEAD~20:libavdevice/x11grab.c-} else { HEAD~20:libavdevice/x11grab.c-memcpy(pal, s->palette, AVPALETTE_SIZE); HEAD~20:libavdevice/x11grab.c-s->palette_changed = 0; HEAD~20:libavdevice/x11grab.c-} HEAD~20:libavdevice/x11grab.c-} -- HEAD~20:libavformat/asfdec_f.c-if (asf_st->pkt.data && asf_st->palette_changed) { HEAD~20:libavformat/asfdec_f.c-uint8_t *pal; HEAD~20:libavformat/asfdec_f.c-pal = av_packet_new_side_data(&asf_st->pkt, AV_PKT_DATA_PALETTE, HEAD~20:libavformat/asfdec_f.c- AVPALETTE_SIZE); HEAD~20:libavformat/asfdec_f.c-if (!pal) { HEAD~20:libavformat/asfdec_f.c:av_log(s, AV_LOG_ERROR, "Cannot append palette to packet\n"); HEAD~20:libavformat/asfdec_f.c-} else { HEAD~20:libavformat/asfdec_f.c-memcpy(pal, asf_st->palette, AVPALETTE_SIZE); HEAD~20:libavformat/asfdec_f.c- asf_st->palette_changed = 0; HEAD~20:libavformat/asfdec_f.c-} HEAD~20:libavformat/asfdec_f.c-} -- HEAD~20:libavformat/mov.c-if (sc->has_palette) { HEAD~20:libavformat/mov.c-uint8_t *pal; HEAD~20:libavformat/mov.c- HEAD~20:libavformat/mov.c-pal = av_packet_new_side_data(pkt, AV_PKT_DATA_PALETTE, AVPALETTE_SIZE); HEAD~20:libavformat/mov.c-if (!pal) { HEAD~20:libavformat/mov.c:av_log(mov->fc, AV_LOG_ERROR, "Cannot append palette to packet\n"); HEAD~20:libavformat/mov.c-} else { HEAD~20:libavformat/mov.c-memcpy(pal, sc->palette, AVPALETTE_SIZE); HEAD~20:libavformat/mov.c-sc->has_palette = 0; HEAD~20:libavformat/mov.c-} HEAD~20:libavformat/mov.c-} [...] ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Interesting ;) By the way, here's a clip from the old patch (THIS one had significant parts written by Carl alright) that got rave reviews from Hendrik Leppkes, calling it "an extremely ugly hack", complaining at the calling into another demuxer (by using the ff_mov_read_stsd_entries() function in mov.c): +priv_data = st->priv_data; +nb_streams = s->nb_streams; +mc = av_mallocz(sizeof(*mc)); +if (! mc) +return AVERROR(ENOMEM); +mc->fc = s; +st->priv_data = msc = av_mallocz(sizeof(MOVStreamContext)); +if (!msc) { +av_free(mc); +st->priv_data = priv_data; +return AVERROR(ENOMEM); +} +ffio_init_context(&pb, track->codec_priv.data, track->codec_priv.size, 0, NULL, NULL, NULL, NULL); +/* ff_mov_read_stsd_entries updates stream s->nb_streams-1, + * so set it temporarily to indicate which stream
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
L'octidi 8 nivôse, an CCXXIV, Mats Peterson a écrit : > By the way, here's a clip from the old patch (THIS one had significant parts > written by Carl alright) that got rave reviews from Hendrik Leppkes, calling > it "an extremely ugly hack", complaining at the calling into another demuxer > (by using the ff_mov_read_stsd_entries() function in mov.c): What is the point of bringing that up? Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On 12/28/2015 03:39 PM, Nicolas George wrote: L'octidi 8 nivôse, an CCXXIV, Michael Niedermayer a écrit : for reference: (similar code prior to the patches) Indeed. Mats already pointed one of them. This would warrant some refactoring. At the very least: ff_frame_new_palette(frame, palette); Then Ganesh will be able to add av_warn_unused_result on it ;) (And in the meantime, it could set AV_FRAME_FLAG_CORRUPT.= Regards, ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel A refactoring indeed sounds sensible to me. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On 12/28/2015 03:45 PM, Nicolas George wrote: L'octidi 8 nivôse, an CCXXIV, Mats Peterson a écrit : By the way, here's a clip from the old patch (THIS one had significant parts written by Carl alright) that got rave reviews from Hendrik Leppkes, calling it "an extremely ugly hack", complaining at the calling into another demuxer (by using the ff_mov_read_stsd_entries() function in mov.c): What is the point of bringing that up? Regards, ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel The point is to make people understand he didn't write "significant parts" of the latest patch. I want to make that clear. Not that I care who wrote it, as long as it works. I just don't like his grabbing the honor for something he didn't write (except for the stuff we already mentioned). Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/http: Documented http_proxy option
On Mon, Dec 28, 2015 at 09:50:15AM +, Joel Holdsworth wrote: > --- > doc/protocols.texi | 3 +++ > 1 file changed, 3 insertions(+) applied thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The bravest are surely those who have the clearest vision of what is before them, glory and danger alike, and yet notwithstanding go out to meet it. -- Thucydides signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On 12/28/2015 03:48 PM, Mats Peterson wrote: On 12/28/2015 03:45 PM, Nicolas George wrote: L'octidi 8 nivôse, an CCXXIV, Mats Peterson a écrit : By the way, here's a clip from the old patch (THIS one had significant parts written by Carl alright) that got rave reviews from Hendrik Leppkes, calling it "an extremely ugly hack", complaining at the calling into another demuxer (by using the ff_mov_read_stsd_entries() function in mov.c): What is the point of bringing that up? Regards, ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel The point is to make people understand he didn't write "significant parts" of the latest patch. I want to make that clear. Not that I care who wrote it, as long as it works. I just don't like his grabbing the honor for something he didn't write (except for the stuff we already mentioned). Mats The "significant parts" were made by me. I have modified both matroskadec.c and mov.c, and I've added a new file qtpalette.c, shared by the previously mentioned files. Once again, I don't care who did it, as long as nobody tries to grab the honor for something he didn't write. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
L'octidi 8 nivôse, an CCXXIV, Mats Peterson a écrit : > The point is to make people understand he didn't write "significant parts" > of the latest patch. I want to make that clear. Not that I care who wrote > it, as long as it works. I just don't like his grabbing the honor for > something he didn't write (except for the stuff we already mentioned). People on this list have known Carl Eugen for a long time, they do not need a new self-appointed prophet to denounce him. And quoting Hendrik's words looks like an attempt at public humiliation. Let us focus on the merits of the patch itself please. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On 12/28/2015 04:04 PM, Nicolas George wrote: L'octidi 8 nivôse, an CCXXIV, Mats Peterson a écrit : The point is to make people understand he didn't write "significant parts" of the latest patch. I want to make that clear. Not that I care who wrote it, as long as it works. I just don't like his grabbing the honor for something he didn't write (except for the stuff we already mentioned). People on this list have known Carl Eugen for a long time, they do not need a new self-appointed prophet to denounce him. And quoting Hendrik's words looks like an attempt at public humiliation. Let us focus on the merits of the patch itself please. Regards, ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel It wasn't meant to appear like public humiliation. I rather wanted to show what the old patch looked like, as the similarities to my patch are close to non-existent. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On 12/28/2015 04:07 PM, Mats Peterson wrote: On 12/28/2015 04:04 PM, Nicolas George wrote: L'octidi 8 nivôse, an CCXXIV, Mats Peterson a écrit : The point is to make people understand he didn't write "significant parts" of the latest patch. I want to make that clear. Not that I care who wrote it, as long as it works. I just don't like his grabbing the honor for something he didn't write (except for the stuff we already mentioned). People on this list have known Carl Eugen for a long time, they do not need a new self-appointed prophet to denounce him. And quoting Hendrik's words looks like an attempt at public humiliation. Let us focus on the merits of the patch itself please. Regards, ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel It wasn't meant to appear like public humiliation. I rather wanted to show what the old patch looked like, as the similarities to my patch are close to non-existent. Mats This is quickly becoming a rather meaningless discussion. The patch works. And the "significant parts" were written by me. That's it. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On 12/28/2015 04:04 PM, Nicolas George wrote: L'octidi 8 nivôse, an CCXXIV, Mats Peterson a écrit : The point is to make people understand he didn't write "significant parts" of the latest patch. I want to make that clear. Not that I care who wrote it, as long as it works. I just don't like his grabbing the honor for something he didn't write (except for the stuff we already mentioned). People on this list have known Carl Eugen for a long time, they do not need a new self-appointed prophet to denounce him. And quoting Hendrik's words looks like an attempt at public humiliation. Let us focus on the merits of the patch itself please. Regards, ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Listen, Nicolas. I'm not DENOUNCING Carl Eugen by saying he didn't write the significant parts of the patch. Where did you get that from? Would YOU like someone else grabbing the honor for something you've written? I don't think you would. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
L'octidi 8 nivôse, an CCXXIV, Mats Peterson a écrit : > Listen, Nicolas. I'm not DENOUNCING Carl Eugen by saying he didn't write the > significant parts of the patch. Where did you get that from? Would YOU like > someone else grabbing the honor for something you've written? I don't think > you would. Michael's quote of previous snippets was enough. There was no need to dig up Hendrik's comment on top of that, especially since it has no relevance when it comes to authorship. Stop hounding on Carl Eugen, it is very annoying. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On 12/28/2015 04:26 PM, Nicolas George wrote: L'octidi 8 nivôse, an CCXXIV, Mats Peterson a écrit : Listen, Nicolas. I'm not DENOUNCING Carl Eugen by saying he didn't write the significant parts of the patch. Where did you get that from? Would YOU like someone else grabbing the honor for something you've written? I don't think you would. Michael's quote of previous snippets was enough. There was no need to dig up Hendrik's comment on top of that, especially since it has no relevance when it comes to authorship. Stop hounding on Carl Eugen, it is very annoying. Regards, So it's OK for him to say he wrote the significant parts of the patch? That's just as annoying. But OK, Michael's proof is enough in a way. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
L'octidi 8 nivôse, an CCXXIV, Mats Peterson a écrit : > So it's OK for him to say he wrote the significant parts of the patch? It's ok for him to say that, it's ok for other people to disagree and prove their point. It's not ok to rehash the question endlessly. It's not ok to dig up irrelevant humiliating quotes. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On 12/28/2015 04:27 PM, Mats Peterson wrote: On 12/28/2015 04:26 PM, Nicolas George wrote: L'octidi 8 nivôse, an CCXXIV, Mats Peterson a écrit : Listen, Nicolas. I'm not DENOUNCING Carl Eugen by saying he didn't write the significant parts of the patch. Where did you get that from? Would YOU like someone else grabbing the honor for something you've written? I don't think you would. Michael's quote of previous snippets was enough. There was no need to dig up Hendrik's comment on top of that, especially since it has no relevance when it comes to authorship. Stop hounding on Carl Eugen, it is very annoying. Regards, So it's OK for him to say he wrote the significant parts of the patch? That's just as annoying. But OK, Michael's proof is enough in a way. Mats Not that they show anything of what I was talking about, though. What Michael pasted was only the part used in matroska_deliver_packet(). Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On 12/28/2015 04:33 PM, Nicolas George wrote: L'octidi 8 nivôse, an CCXXIV, Mats Peterson a écrit : So it's OK for him to say he wrote the significant parts of the patch? It's ok for him to say that, it's ok for other people to disagree and prove their point. It's not ok to rehash the question endlessly. It's not ok to dig up irrelevant humiliating quotes. Regards, ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Well, I guess I have to temper myself a bit. But I don't accept that someone else grabs the honor for something I've written. Would you, once again? Can you understand my point whatsoever? Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
L'octidi 8 nivôse, an CCXXIV, Mats Peterson a écrit : > Well, I guess I have to temper myself a bit. But I don't accept that someone > else grabs the honor for something I've written. Would you, once again? Can > you understand my point whatsoever? I have seen your point. I had already seen your point twenty messages ago if not more, and that is exactly the problem I am complaining about, there was no need for this endless rehashing of the issue. A mailing-list is not tweeter nor IRC. Maybe you should give you a rule to wait thirty minutes between reading a message and replying to it and another thirty minutes between writing the reply and sending it or something. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On 12/28/2015 04:41 PM, Nicolas George wrote: L'octidi 8 nivôse, an CCXXIV, Mats Peterson a écrit : Well, I guess I have to temper myself a bit. But I don't accept that someone else grabs the honor for something I've written. Would you, once again? Can you understand my point whatsoever? I have seen your point. I had already seen your point twenty messages ago if not more, and that is exactly the problem I am complaining about, there was no need for this endless rehashing of the issue. A mailing-list is not tweeter nor IRC. Maybe you should give you a rule to wait thirty minutes between reading a message and replying to it and another thirty minutes between writing the reply and sending it or something. Regards, ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Alright. Sorry for that. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On Mon, Dec 28, 2015 at 6:39 AM, Nicolas George wrote: > L'octidi 8 nivôse, an CCXXIV, Michael Niedermayer a écrit : >> for reference: (similar code prior to the patches) > > Indeed. Mats already pointed one of them. This would warrant some > refactoring. At the very least: > > ff_frame_new_palette(frame, palette); > > Then Ganesh will be able to add av_warn_unused_result on it ;) (And in the > meantime, it could set AV_FRAME_FLAG_CORRUPT.= :D. It would be great if we had a way to track these; I may have easily missed this message, given the huge volume of messages on this thread. Noted in my todo list, thanks. @Mats: this is a good example for why it is very helpful to reduce the volume of email messages, and to think over them before sending to avoid small additions/removals/clarifications. > > Regards, > > -- > Nicolas George > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] lavfi/af_anequalizer: replace pow(x, -2) by 1/(x*x)
On Mon, Dec 28, 2015 at 12:14 AM, Paul B Mahol wrote: > On 12/28/15, Ganesh Ajjanagadde wrote: >> Signed-off-by: Ganesh Ajjanagadde >> --- >> libavfilter/af_anequalizer.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/libavfilter/af_anequalizer.c b/libavfilter/af_anequalizer.c >> index d7b5b6c..649c0b9 100644 >> --- a/libavfilter/af_anequalizer.c >> +++ b/libavfilter/af_anequalizer.c >> @@ -391,8 +391,8 @@ static void chebyshev1_bp_filter(EqualizatorFilter *f, >> >> epsilon = sqrt((G*G - Gb*Gb) / (Gb*Gb - G0*G0)); >> g0 = pow(G0,1.0/N); >> -alfa = pow(1.0/epsilon+ sqrt(1 + pow(epsilon,-2.0)), 1.0/N); >> -beta = pow(G/epsilon + Gb * sqrt(1 + pow(epsilon,-2.0)), 1.0/N); >> +alfa = pow(1.0/epsilon+ sqrt(1 + 1/(epsilon*epsilon)), 1.0/N); >> +beta = pow(G/epsilon + Gb * sqrt(1 + 1/(epsilon*epsilon)), 1.0/N); >> a = 0.5 * (alfa - 1.0/alfa); >> b = 0.5 * (beta - g0*g0*(1/beta)); >> tetta_b = tan(wb/2); >> -- >> 2.6.4 >> >> ___ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> > > both patches ok thanks, please also have a look at the one removing cabs/cexp: https://ffmpeg.org/pipermail/ffmpeg-devel/2015-December/185847.html. Pushing later today. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavfi/af_anequalizer: remove cabs, cexp dependencies
On 12/27/15, Ganesh Ajjanagadde wrote: > Replaces by real arithmetic. Tested the validity of these transformations > separately. > > Signed-off-by: Ganesh Ajjanagadde > --- > configure| 1 - > libavfilter/af_anequalizer.c | 25 + > 2 files changed, 17 insertions(+), 9 deletions(-) > If those are mathematically equivalent ok. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [RFC 3/3] oggenc: add support for remuxing Daala video into OGG container
Signed-off-by: Rostislav Pehlivanov --- libavformat/oggenc.c | 27 +-- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/libavformat/oggenc.c b/libavformat/oggenc.c index 4907512..87dcdde 100644 --- a/libavformat/oggenc.c +++ b/libavformat/oggenc.c @@ -452,7 +452,8 @@ static int ogg_write_header(AVFormatContext *s) st->codec->codec_id != AV_CODEC_ID_THEORA && st->codec->codec_id != AV_CODEC_ID_SPEEX && st->codec->codec_id != AV_CODEC_ID_FLAC && -st->codec->codec_id != AV_CODEC_ID_OPUS) { +st->codec->codec_id != AV_CODEC_ID_OPUS && +st->codec->codec_id != AV_CODEC_ID_DAALA) { av_log(s, AV_LOG_ERROR, "Unsupported codec id in stream %d\n", i); return AVERROR(EINVAL); } @@ -510,19 +511,25 @@ static int ogg_write_header(AVFormatContext *s) } } else { uint8_t *p; -const char *cstr = st->codec->codec_id == AV_CODEC_ID_VORBIS ? "vorbis" : "theora"; -int header_type = st->codec->codec_id == AV_CODEC_ID_VORBIS ? 3 : 0x81; -int framing_bit = st->codec->codec_id == AV_CODEC_ID_VORBIS ? 1 : 0; - -if (avpriv_split_xiph_headers(st->codec->extradata, st->codec->extradata_size, - st->codec->codec_id == AV_CODEC_ID_VORBIS ? 30 : 42, - (const uint8_t**)oggstream->header, oggstream->header_len) < 0) { +enum AVCodecID cid = st->codec->codec_id; +const char *cstr = cid == AV_CODEC_ID_VORBIS ? "vorbis" : + cid == AV_CODEC_ID_DAALA ? "daala" : "theora"; +int f_hdr_size = cid == AV_CODEC_ID_VORBIS ? 30 : + cid == AV_CODEC_ID_DAALA ? 47 : 42; +int header_type = cid == AV_CODEC_ID_VORBIS ? 3 : 0x81; +int framing_bit = cid == AV_CODEC_ID_VORBIS ? 1 : 0; + +if (avpriv_split_xiph_headers(st->codec->extradata, + st->codec->extradata_size, + f_hdr_size, + (const uint8_t**)oggstream->header, + oggstream->header_len) < 0) { av_log(s, AV_LOG_ERROR, "Extradata corrupted\n"); av_freep(&st->priv_data); return AVERROR_INVALIDDATA; } -p = ogg_write_vorbiscomment(7, s->flags & AVFMT_FLAG_BITEXACT, +p = ogg_write_vorbiscomment(strlen(cstr), s->flags & AVFMT_FLAG_BITEXACT, &oggstream->header_len[1], &st->metadata, framing_bit); oggstream->header[1] = p; @@ -530,7 +537,7 @@ static int ogg_write_header(AVFormatContext *s) return AVERROR(ENOMEM); bytestream_put_byte(&p, header_type); -bytestream_put_buffer(&p, cstr, 6); +bytestream_put_buffer(&p, cstr, strlen(cstr)); if (st->codec->codec_id == AV_CODEC_ID_THEORA) { /** KFGSHIFT is the width of the less significant section of the granule position -- 2.6.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [RFC 2/3] daala_parser: add a Daala parser
Not much to say about this other than "it works". Signed-off-by: Rostislav Pehlivanov --- configure | 1 + libavcodec/Makefile | 1 + libavcodec/allcodecs.c| 1 + libavcodec/daala_parser.c | 50 +++ 4 files changed, 53 insertions(+) create mode 100644 libavcodec/daala_parser.c diff --git a/configure b/configure index a510e8f..e84da27 100755 --- a/configure +++ b/configure @@ -2275,6 +2275,7 @@ comfortnoise_encoder_select="lpc" cook_decoder_select="audiodsp mdct sinewin" cscd_decoder_select="lzo" cscd_decoder_suggest="zlib" +daala_decoder_select="daala_parser" dca_decoder_select="fmtconvert mdct" dds_decoder_select="texturedsp" dirac_decoder_select="dirac_parse dwt golomb videodsp mpegvideoenc" diff --git a/libavcodec/Makefile b/libavcodec/Makefile index 85d2e00..b642d2f 100644 --- a/libavcodec/Makefile +++ b/libavcodec/Makefile @@ -864,6 +864,7 @@ OBJS-$(CONFIG_BMP_PARSER) += bmp_parser.o OBJS-$(CONFIG_CAVSVIDEO_PARSER)+= cavs_parser.o OBJS-$(CONFIG_COOK_PARSER) += cook_parser.o OBJS-$(CONFIG_DCA_PARSER) += dca_parser.o dca.o +OBJS-$(CONFIG_DAALA_PARSER)+= daala_parser.o OBJS-$(CONFIG_DIRAC_PARSER)+= dirac_parser.o OBJS-$(CONFIG_DNXHD_PARSER)+= dnxhd_parser.o OBJS-$(CONFIG_DPX_PARSER) += dpx_parser.o diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c index 43d99b3..804c076 100644 --- a/libavcodec/allcodecs.c +++ b/libavcodec/allcodecs.c @@ -619,6 +619,7 @@ void avcodec_register_all(void) REGISTER_PARSER(CAVSVIDEO, cavsvideo); REGISTER_PARSER(COOK, cook); REGISTER_PARSER(DCA,dca); +REGISTER_PARSER(DAALA, daala); REGISTER_PARSER(DIRAC, dirac); REGISTER_PARSER(DNXHD, dnxhd); REGISTER_PARSER(DPX,dpx); diff --git a/libavcodec/daala_parser.c b/libavcodec/daala_parser.c new file mode 100644 index 000..5e49061 --- /dev/null +++ b/libavcodec/daala_parser.c @@ -0,0 +1,50 @@ +/* + * Daala video decoder + * + * Copyright (C) 2015 Rostislav Pehlivanov + * + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include "parser.h" +#include "daala_entropy.h" + +static int parse(AVCodecParserContext *s, + AVCodecContext *avctx, + const uint8_t **poutbuf, int *poutbuf_size, + const uint8_t *buf, int buf_size) +{ +DaalaEntropy e; +DaalaBitstreamHeader h; + +daalaent_decode_init(&e, buf, buf_size); +daalaent_decode_frame_header(&e, &h, 0); + +s->key_frame = h.key_frame; +s->pict_type = h.key_frame ? AV_PICTURE_TYPE_I : !h.bipred ? + AV_PICTURE_TYPE_P : AV_PICTURE_TYPE_B; + +*poutbuf = buf; +*poutbuf_size = buf_size; + +return buf_size; +} + +AVCodecParser ff_daala_parser = { +.codec_ids= { AV_CODEC_ID_DAALA }, +.parser_parse = parse, +}; -- 2.6.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC 1/3] daaladec: Implement a native Daala decoder
Rostislav Pehlivanov gmail.com> writes: > The current decoder is able to decode lossless Daala > encoded files (the output of this decoder is still > identical to the output) to the input? > Most of the code here was written using libdaala as > a reference, after optimizing, refactoring and > shortening. As such, although very few lines have > been copied directly, the code is still licensed > under the BSD 2-clause like libdaala. Files which > are completely original work are licensed under the > same license as FFmpeg. I'm not too concerned about > those, so if people want I wouldn't mind licensing > them as a BSD 2-clause like the rest of the decoder. It is your decision (and definitely not mine) but I would suggest you choose one license (either the license of libdaala or the LGPL) and use that for all related files. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] x86/vf_stereo3d: make ff_anaglyph_sse4 work on x86_32
On 12/28/2015 5:15 AM, Paul B Mahol wrote: > On 12/27/15, James Almer wrote: >> Signed-off-by: James Almer >> --- >> libavfilter/x86/vf_stereo3d.asm| 47 >> +++--- >> libavfilter/x86/vf_stereo3d_init.c | 2 +- >> 2 files changed, 45 insertions(+), 4 deletions(-) >> >> diff --git a/libavfilter/x86/vf_stereo3d.asm >> b/libavfilter/x86/vf_stereo3d.asm >> index 29a8c56..491579f 100644 >> --- a/libavfilter/x86/vf_stereo3d.asm >> +++ b/libavfilter/x86/vf_stereo3d.asm >> @@ -22,8 +22,6 @@ >> >> %include "libavutil/x86/x86util.asm" >> >> -%if ARCH_X86_64 >> - >> SECTION_RODATA >> >> ; rgbrgbrgbrgb >> @@ -37,10 +35,33 @@ ex_b: db 2,-1,-1,-1,5,-1,-1,-1,8,-1,-1,-1,11,-1,-1,-1 >> SECTION .text >> >> INIT_XMM sse4 >> +%if ARCH_X86_64 >> cglobal anaglyph, 6, 10, 14, 2*6*mmsize, dst, lsrc, rsrc, dst_linesize, >> l_linesize, r_linesize, width, height, o, cnt >> %define ana_matrix_rq r6q >> %define ana_matrix_gq r7q >> %define ana_matrix_bq r8q >> + >> +%else ; ARCH_X86_32 >> +%if HAVE_ALIGNED_STACK >> +cglobal anaglyph, 3, 7, 8, 2*9*mmsize, dst, lsrc, rsrc, dst_linesize, >> l_linesize, o, cnt >> +%else >> +cglobal anaglyph, 3, 6, 8, 2*9*mmsize, dst, lsrc, rsrc, dst_linesize, o, >> cnt >> +%define l_linesizeq r4mp >> +%endif ; HAVE_ALIGNED_STACK >> +%define ana_matrix_rq r3q >> +%define ana_matrix_gq r4q >> +%define ana_matrix_bq r5q >> +%define r_linesizeq r5mp >> +%define widthd r6mp >> +%define heightd r7mp >> +%define m8 [rsp+mmsize*12] >> +%define m9 [rsp+mmsize*13] >> +%define m10 [rsp+mmsize*14] >> +%define m11 [rsp+mmsize*15] >> +%define m12 [rsp+mmsize*16] >> +%define m13 [rsp+mmsize*17] >> +%endif ; ARCH >> + >> movana_matrix_rq, r8m >> movana_matrix_gq, r9m >> movana_matrix_bq, r10m >> @@ -74,6 +95,7 @@ cglobal anaglyph, 6, 10, 14, 2*6*mmsize, dst, lsrc, rsrc, >> dst_linesize, l_linesi >> mova [rsp+mmsize*10], m4 >> mova [rsp+mmsize*11], m5 >> >> +%if ARCH_X86_64 >> movu m11, [ana_matrix_bq+ 0] >> movq m13, [ana_matrix_bq+16] >> pshufdm8, m11, q >> @@ -84,6 +106,26 @@ cglobal anaglyph, 6, 10, 14, 2*6*mmsize, dst, lsrc, >> rsrc, dst_linesize, l_linesi >> pshufd m13, m13, q >> mov widthd, dword widthm >> mov heightd, dword heightm >> +%else >> +movu m3, [ana_matrix_bq+ 0] >> +movq m5, [ana_matrix_bq+16] >> +pshufdm0, m3, q >> +pshufdm1, m3, q >> +pshufdm2, m3, q >> +pshufdm3, m3, q >> +pshufdm4, m5, q >> +pshufdm5, m5, q >> +mova [rsp+mmsize*12], m0 >> +mova [rsp+mmsize*13], m1 >> +mova [rsp+mmsize*14], m2 >> +mova [rsp+mmsize*15], m3 >> +mova [rsp+mmsize*16], m4 >> +mova [rsp+mmsize*17], m5 >> +movdst_linesizeq, r3m >> +%if HAVE_ALIGNED_STACK >> +mov l_linesizeq, r4m >> +%endif >> +%endif ; ARCH >> >> .nextrow: >> mov od, widthd >> @@ -172,4 +214,3 @@ cglobal anaglyph, 6, 10, 14, 2*6*mmsize, dst, lsrc, >> rsrc, dst_linesize, l_linesi >> sub heightd, 1 >> jg .nextrow >> REP_RET >> -%endif >> diff --git a/libavfilter/x86/vf_stereo3d_init.c >> b/libavfilter/x86/vf_stereo3d_init.c >> index 77d4f7b..da160a8 100644 >> --- a/libavfilter/x86/vf_stereo3d_init.c >> +++ b/libavfilter/x86/vf_stereo3d_init.c >> @@ -31,7 +31,7 @@ void ff_stereo3d_init_x86(Stereo3DDSPContext *dsp) >> { >> int cpu_flags = av_get_cpu_flags(); >> >> -if (ARCH_X86_64 && EXTERNAL_SSE4(cpu_flags)) { >> +if (EXTERNAL_SSE4(cpu_flags)) { >> dsp->anaglyph = ff_anaglyph_sse4; >> } >> } >> -- >> 2.6.3 >> >> ___ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> > > both patches ok if fate is not broken by this Pushed then, thanks. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] support for reading / writing encrypted MP4 files
On Mon, Dec 28, 2015 at 05:29:35AM +, Eran Kornblau wrote: > Bumping up this thread (latest patch attached) > > Happy holidays ! happy holidays as well! > > Eran > [...] > @@ -4007,6 +4009,162 @@ static int mov_read_free(MOVContext *c, AVIOContext > *pb, MOVAtom atom) > return 0; > } > > +static int mov_read_frma(MOVContext *c, AVIOContext *pb, MOVAtom atom) > +{ > +uint32_t format = avio_rl32(pb); > +MOVStreamContext *sc; > +enum AVCodecID id; > +AVStream *st; > + > +if (c->fc->nb_streams < 1) > +return 0; > +st = c->fc->streams[c->fc->nb_streams - 1]; > +sc = st->priv_data; > + > +switch (sc->format) > +{ > +case MKTAG('e','n','c','v'):// encrypted video > +case MKTAG('e','n','c','a'):// encrypted audio > +id = mov_codec_id(st, format); > +st->codec->codec_id = id; this seems missing a check for st->codec->codec_id being "unset" before setting it > +sc->format = format; > +break; > + > +default: > +av_log(c->fc, AV_LOG_WARNING, > + "ignoring 'frma' atom of '%.4s', stream format is '%.4s'\n", > + (char*)&format, (char*)&sc->format); the way these are printed would lead to different results on big and little endian > +break; > +} > + > +return 0; > +} > + > +static int mov_read_senc(MOVContext *c, AVIOContext *pb, MOVAtom atom) > +{ > +AVStream *st; > +MOVStreamContext *sc; > +size_t auxiliary_info_size; > +int ret; > + > +if (c->decryption_key_len == 0 || c->fc->nb_streams < 1) > +return 0; > + > +st = c->fc->streams[c->fc->nb_streams - 1]; > +sc = st->priv_data; > + > +if (sc->cenc.aes_ctr) { > +av_log(c->fc, AV_LOG_ERROR, "duplicate senc atom\n"); > +return AVERROR_INVALIDDATA; > +} > + > +avio_r8(pb); /* version */ > +sc->cenc.use_subsamples = avio_rb24(pb) & 0x02; /* flags */ > + > +avio_rb32(pb);/* entries */ > + > +if (atom.size < 8) { > +av_log(c->fc, AV_LOG_ERROR, "senc atom size %"PRId64" too small\n", > atom.size); > +return AVERROR_INVALIDDATA; > +} > + > +/* save the auxiliary info as is */ > +auxiliary_info_size = atom.size - 8; > + > +sc->cenc.auxiliary_info = av_malloc(auxiliary_info_size); > +if (!sc->cenc.auxiliary_info) { > +return AVERROR(ENOMEM); > +} > + > +sc->cenc.auxiliary_info_end = sc->cenc.auxiliary_info + > auxiliary_info_size; > + > +sc->cenc.auxiliary_info_pos = sc->cenc.auxiliary_info; > + > +if (avio_read(pb, sc->cenc.auxiliary_info, auxiliary_info_size) != > auxiliary_info_size) { > +av_log(c->fc, AV_LOG_ERROR, "failed to read the auxiliary info"); > +return AVERROR_INVALIDDATA; > +} > + > +/* initialize the cipher */ > +sc->cenc.aes_ctr = av_aes_ctr_alloc(); > +if (!sc->cenc.aes_ctr) { > +return AVERROR(ENOMEM); > +} > + > +ret = av_aes_ctr_init(sc->cenc.aes_ctr, c->decryption_key); > +if (ret) { > +return ret; > +} > + > +return 0; this could be simplfied also errors in ffmpeg are generally negative so if (ret < 0) seems more correct if a seperate check is left > +} > + > +static int cenc_filter(MOVContext *c, MOVStreamContext *sc, uint8_t *input, > int size) > +{ > +uint32_t encrypted_bytes; > +uint16_t subsample_count; > +uint16_t clear_bytes; > +uint8_t* input_end = input + size; > + > +/* read the iv */ > +if (sc->cenc.auxiliary_info_pos + AES_CTR_IV_SIZE > > sc->cenc.auxiliary_info_end) { > +av_log(c->fc, AV_LOG_ERROR, "failed to read iv from the auxiliary > info\n"); > +return AVERROR_INVALIDDATA; > +} > + > +av_aes_ctr_set_iv(sc->cenc.aes_ctr, sc->cenc.auxiliary_info_pos); > +sc->cenc.auxiliary_info_pos += AES_CTR_IV_SIZE; > + > +if (!sc->cenc.use_subsamples) > +{ > +/* decrypt the whole packet */ > +av_aes_ctr_crypt(sc->cenc.aes_ctr, input, input, size); > +return 0; > +} > + > +/* read the subsample count */ > +if (sc->cenc.auxiliary_info_pos + sizeof(uint16_t) > > sc->cenc.auxiliary_info_end) { the addition could overflow, which would be undefined behavior something like: sizeof(uint16_t) > sc->cenc.auxiliary_info_end - sc->cenc.auxiliary_info_pos is correcter > +av_log(c->fc, AV_LOG_ERROR, "failed to read subsample count from the > auxiliary info\n"); > +return AVERROR_INVALIDDATA; > +} > + > +subsample_count = AV_RB16(sc->cenc.auxiliary_info_pos); > +sc->cenc.auxiliary_info_pos += sizeof(uint16_t); > + > +for (; subsample_count > 0; subsample_count--) > +{ > +if (sc->cenc.auxiliary_info_pos + 6 > sc->cenc.auxiliary_info_end) { > +av_log(c->fc, AV_LOG_ERROR, "failed to read subsample from the > auxiliary info\n"); > +return AVERROR_INVALIDDATA; > +} > + > +/* read the number of cl
Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska
On 12/28/2015 04:58 PM, Ganesh Ajjanagadde wrote: @Mats: this is a good example for why it is very helpful to reduce the volume of email messages, and to think over them before sending to avoid small additions/removals/clarifications. Yes, i can fully understand that. I'm going to behave in the future. Mats ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] doc: make apidoc output independent of SRC_PATH
On 20.12.2015 23:31, Andreas Cadhalpun wrote: > Previously it included the SRC_PATH in every title. > > Signed-off-by: Andreas Cadhalpun > --- > doc/Makefile| 7 --- > doc/doxy-wrapper.sh | 11 ++- > 2 files changed, 10 insertions(+), 8 deletions(-) > > diff --git a/doc/Makefile b/doc/Makefile > index 3e67c2a..afa7e22 100644 > --- a/doc/Makefile > +++ b/doc/Makefile > @@ -124,11 +124,12 @@ $(DOCS) doc/doxy/html: | doc/ > $(DOC_EXAMPLES:%$(EXESUF)=%.o): | doc/examples > OBJDIRS += doc/examples > > -DOXY_INPUT = $(addprefix $(SRC_PATH)/, $(INSTHEADERS) > $(DOC_EXAMPLES:%$(EXESUF)=%.c) $(LIB_EXAMPLES:%$(EXESUF)=%.c)) > +DOXY_INPUT = $(INSTHEADERS) $(DOC_EXAMPLES:%$(EXESUF)=%.c) > $(LIB_EXAMPLES:%$(EXESUF)=%.c) > +DOXY_INPUT_DEPS = $(addprefix $(SRC_PATH)/, $(DOXY_INPUT)) > > doc/doxy/html: TAG = DOXY > -doc/doxy/html: $(SRC_PATH)/doc/Doxyfile $(SRC_PATH)/doc/doxy-wrapper.sh > $(DOXY_INPUT) > - $(M)$(SRC_PATH)/doc/doxy-wrapper.sh $(SRC_PATH) $< $(DOXYGEN) > $(DOXY_INPUT) > +doc/doxy/html: $(SRC_PATH)/doc/Doxyfile $(SRC_PATH)/doc/doxy-wrapper.sh > $(DOXY_INPUT_DEPS) > + OUT_DIR=$$PWD/doc/doxy; cd $(SRC_PATH); $(M)./doc/doxy-wrapper.sh > $$OUT_DIR $< $(DOXYGEN) $(DOXY_INPUT); > > install-doc: install-html install-man > > diff --git a/doc/doxy-wrapper.sh b/doc/doxy-wrapper.sh > index 9720e54..0c02777 100755 > --- a/doc/doxy-wrapper.sh > +++ b/doc/doxy-wrapper.sh > @@ -1,21 +1,22 @@ > #!/bin/sh > > -SRC_PATH="${1}" > +OUT_DIR="${1}" > DOXYFILE="${2}" > DOXYGEN="${3}" > > shift 3 > > -if [ -e "$SRC_PATH/VERSION" ]; then > -VERSION=`cat "$SRC_PATH/VERSION"` > +if [ -e "VERSION" ]; then > +VERSION=`cat "VERSION"` > else > -VERSION=`cd "$SRC_PATH"; git describe` > +VERSION=`git describe` > fi > > $DOXYGEN - < @INCLUDE= ${DOXYFILE} > INPUT = $@ > -EXAMPLE_PATH= ${SRC_PATH}/doc/examples > +EXAMPLE_PATH= doc/examples > HTML_TIMESTAMP = NO > PROJECT_NUMBER = $VERSION > +OUTPUT_DIRECTORY = $OUT_DIR > EOF > Ping. Best regards, Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] build: make out-of-tree builds bit-identical to in-tree builds
Previously the full source path was embedded inconsistently in the debug information between in-tree/out-of-tree builds. The 'vpath %.inc' becomes necessary for finding libavfilter/all_channel_layouts.inc in out-of-tree builds. The full source path is still embedded in the debug information, but it's now independent of whether building in-tree or out-of-tree. The biggest improvement of this patch is that gdb now always searches for the path relative to the source directory. It still also searches for the full path. Previously it searched only for the full path in out-of-tree builds, making the debug information generated by Debian's buildds rather hard to use. Signed-off-by: Andreas Cadhalpun --- Makefile| 1 + common.mak | 6 +++--- configure | 1 + library.mak | 2 +- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 7836a20..f3bd5f6 100644 --- a/Makefile +++ b/Makefile @@ -4,6 +4,7 @@ include config.mak vpath %.c$(SRC_PATH) vpath %.cpp $(SRC_PATH) vpath %.h$(SRC_PATH) +vpath %.inc $(SRC_PATH) vpath %.m$(SRC_PATH) vpath %.S$(SRC_PATH) vpath %.asm $(SRC_PATH) diff --git a/common.mak b/common.mak index c3a8b93..bad2627 100644 --- a/common.mak +++ b/common.mak @@ -32,7 +32,7 @@ endif ALLFFLIBS = avcodec avdevice avfilter avformat avresample avutil postproc swscale swresample # NASM requires -I path terminated with / -IFLAGS := -I. -I$(SRC_PATH)/ +IFLAGS := -I$(DST_PATH)/ -I$(SRC_PATH)/ CPPFLAGS := $(IFLAGS) $(CPPFLAGS) CFLAGS += $(ECFLAGS) CCFLAGS = $(CPPFLAGS) $(CFLAGS) @@ -43,11 +43,11 @@ CXXFLAGS += $(CPPFLAGS) $(CFLAGS) YASMFLAGS += $(IFLAGS:%=%/) -Pconfig.asm HOSTCCFLAGS = $(IFLAGS) $(HOSTCPPFLAGS) $(HOSTCFLAGS) -LDFLAGS:= $(ALLFFLIBS:%=$(LD_PATH)lib%) $(LDFLAGS) +LDFLAGS:= $(ALLFFLIBS:%=$(LD_PATH)$(DST_PATH)/lib%) $(LDFLAGS) define COMPILE $(call $(1)DEP,$(1)) - $($(1)) $($(1)FLAGS) $($(1)_DEPFLAGS) $($(1)_C) $($(1)_O) $< + $(Q)cd $(SRC_PATH); $(subst @,,$($(1))) $($(1)FLAGS) $($(1)_DEPFLAGS:$(@:.o=.d)=$(DST_PATH)/$(@:.o=.d)) $($(1)_C) $($(1)_O:$@=$(DST_PATH)/$@) $(subst $(SRC_PATH)/,,$<) endef COMPILE_C = $(call COMPILE,CC) diff --git a/configure b/configure index a510e8f..f604edd 100755 --- a/configure +++ b/configure @@ -6205,6 +6205,7 @@ SRC_PATH=$source_path ifndef MAIN_MAKEFILE SRC_PATH:=\$(SRC_PATH:.%=..%) endif +DST_PATH=$(pwd) CC_IDENT=$cc_ident ARCH=$arch INTRINSICS=$intrinsics diff --git a/library.mak b/library.mak index 6f95f63..0b23a28 100644 --- a/library.mak +++ b/library.mak @@ -28,7 +28,7 @@ $(SUBDIR)x86/%$(DEFAULT_YASMD).asm: $(SUBDIR)x86/%.asm $(SUBDIR)x86/%.o: $(SUBDIR)x86/%$(YASMD).asm $(DEPYASM) $(YASMFLAGS) -I $( $(@:.o=.d) - $(YASM) $(YASMFLAGS) -I $(http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] support for reading / writing encrypted MP4 files
> > +case MKTAG('e','n','c','v'):// encrypted video > > +case MKTAG('e','n','c','a'):// encrypted audio > > +id = mov_codec_id(st, format); > > +st->codec->codec_id = id; > > this seems missing a check for st->codec->codec_id being "unset" > before setting it > Will add it > > > +sc->format = format; > > +break; > > + > > +default: > > +av_log(c->fc, AV_LOG_WARNING, > > + "ignoring 'frma' atom of '%.4s', stream format is '%.4s'\n", > > + (char*)&format, (char*)&sc->format); > > the way these are printed would lead to different results on big and > little endian > Correct, however it's already done this way in a few other places in this file (search for %.4s) Do you prefer that I will print it with %c%c%c%c as in ff_mov_read_stsd_entries ? > > + > > +ret = av_aes_ctr_init(sc->cenc.aes_ctr, c->decryption_key); > > +if (ret) { > > +return ret; > > +} > > + > > +return 0; > > this could be simplfied > also errors in ffmpeg are generally negative so > if (ret < 0) seems more correct if a seperate check is left > Ok, will change to 'return av_aes_ctr_init(...)' > > > +/* read the subsample count */ > > +if (sc->cenc.auxiliary_info_pos + sizeof(uint16_t) > > > sc->cenc.auxiliary_info_end) { > > the addition could overflow, which would be undefined behavior > something like: > sizeof(uint16_t) > sc->cenc.auxiliary_info_end - sc->cenc.auxiliary_info_pos > is correcter > Will change to what you suggested > > +if (input + clear_bytes + encrypted_bytes > input_end) { > > the additions can overflow > Will change to: if ((uint64_t)clear_bytes + encrypted_bytes > input_end - input) { > [...] > > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > Thanks ! Eran ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] ffplay: insertion point of the auto rotation filter - Github ticket #141
On Mon, 28 Dec 2015, Michael Niedermayer wrote: On Sun, Dec 27, 2015 at 11:09:42PM +0100, Balint Marton wrote: Hi Michael, The patch you committed seems to break the cropping to even width / height as required by SDL overlay code. Also there can be a use case for inserting the auto rotation filter after the user provided filter chain as well. (e,g, deinterlacing). do you have a sample which uses interlacing and rotation ? on second thought this seems a somewhat odd combination rotated videos are something i would associate with phones, interlacing i would generally not associate with phones No, I don't have such a sample, you're right, probably it is not common. But I guess there are other - more commonly used - examples. Another reason why I prefer things the way they are now is that if eventually ffplay will support SDL2, rotation will be cheap, because the video will be a proper texture, and this also means that auto rotation will be done at the end of the filter chain... Regards, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avfilter/avf_showspectrum: properly calculate w factor
It is used in calculating loudness of each frequency bin. This increases contrast for all scale modes except log. Signed-off-by: Paul B Mahol --- libavfilter/avf_showspectrum.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/libavfilter/avf_showspectrum.c b/libavfilter/avf_showspectrum.c index 55cc668..ff991e0 100644 --- a/libavfilter/avf_showspectrum.c +++ b/libavfilter/avf_showspectrum.c @@ -57,6 +57,7 @@ typedef struct { FFTSample **rdft_data; ///< bins holder for each (displayed) channels float *window_func_lut; ///< Window function LUT int win_func; +double win_scale; float overlap; float *combine_buffer; ///< color combining buffer (3 * h items) } ShowSpectrumContext; @@ -218,6 +219,10 @@ static int config_output(AVFilterLink *outlink) return AVERROR(ENOMEM); ff_generate_window_func(s->window_func_lut, win_size, s->win_func, &s->overlap); +for (s->win_scale = 0, i = 0; i < win_size; i++) { +s->win_scale += s->window_func_lut[i] * s->window_func_lut[i]; +} + /* prepare the initial picref buffer (black frame) */ av_frame_free(&s->outpicref); s->outpicref = outpicref = @@ -285,7 +290,7 @@ static int plot_spectrum_column(AVFilterLink *inlink, AVFrame *insamples) * height (or half the RDFT window size) */ const int nb_freq = 1 << (s->rdft_bits - 1); const int win_size = nb_freq << 1; -const double w = 1. / (sqrt(nb_freq) * 32768.); +const double w = 1. / (s->win_scale * s->win_scale); int h = s->channel_height; int ch, plane, n, y; -- 1.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavfi/af_anequalizer: remove cabs, cexp dependencies
On Mon, Dec 28, 2015 at 8:22 AM, Paul B Mahol wrote: > On 12/27/15, Ganesh Ajjanagadde wrote: >> Replaces by real arithmetic. Tested the validity of these transformations >> separately. >> >> Signed-off-by: Ganesh Ajjanagadde >> --- >> configure| 1 - >> libavfilter/af_anequalizer.c | 25 + >> 2 files changed, 17 insertions(+), 9 deletions(-) >> > > If those are mathematically equivalent ok. mathematically equivalent, tested offline, numerical differences of the order of ~ 1e-15 or so, which is perfectly fine IMHO for double precision especially since it is a priori not clear which is more precise. Pushed, thanks. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] lavfi/af_anequalizer: replace pow(x, -2) by 1/(x*x)
On Mon, Dec 28, 2015 at 8:00 AM, Ganesh Ajjanagadde wrote: > On Mon, Dec 28, 2015 at 12:14 AM, Paul B Mahol wrote: >> On 12/28/15, Ganesh Ajjanagadde wrote: >>> Signed-off-by: Ganesh Ajjanagadde >>> --- >>> libavfilter/af_anequalizer.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/libavfilter/af_anequalizer.c b/libavfilter/af_anequalizer.c >>> index d7b5b6c..649c0b9 100644 >>> --- a/libavfilter/af_anequalizer.c >>> +++ b/libavfilter/af_anequalizer.c >>> @@ -391,8 +391,8 @@ static void chebyshev1_bp_filter(EqualizatorFilter *f, >>> >>> epsilon = sqrt((G*G - Gb*Gb) / (Gb*Gb - G0*G0)); >>> g0 = pow(G0,1.0/N); >>> -alfa = pow(1.0/epsilon+ sqrt(1 + pow(epsilon,-2.0)), 1.0/N); >>> -beta = pow(G/epsilon + Gb * sqrt(1 + pow(epsilon,-2.0)), 1.0/N); >>> +alfa = pow(1.0/epsilon+ sqrt(1 + 1/(epsilon*epsilon)), 1.0/N); >>> +beta = pow(G/epsilon + Gb * sqrt(1 + 1/(epsilon*epsilon)), 1.0/N); >>> a = 0.5 * (alfa - 1.0/alfa); >>> b = 0.5 * (beta - g0*g0*(1/beta)); >>> tetta_b = tan(wb/2); >>> -- >>> 2.6.4 >>> >>> ___ >>> ffmpeg-devel mailing list >>> ffmpeg-devel@ffmpeg.org >>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>> >> >> both patches ok > > thanks, please also have a look at the one removing cabs/cexp: > https://ffmpeg.org/pipermail/ffmpeg-devel/2015-December/185847.html. > Pushing later today. pushed ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC 1/3] daaladec: Implement a native Daala decoder
On Mon, Dec 28, 2015 at 04:58:48PM +, Rostislav Pehlivanov wrote: > This commit is a work in progress Daala decoder implemented in FFmpeg. > Comments are requested. > > The current decoder is able to decode lossless Daala encoded files (the > output of this decoder is still identical to the output) and Daala > I-frames at any size and quantization level. > > Performance wise, this decoder is around 1.8 times faster than the > reference decoder and is able to open images larger than the > player_example provided with libdaala (which apparently runs out of > memory). > > There's a missing filter so the output will look subjectively blockier > than the reference decoder at higher quantization levels (which blurs > the hell out of any detail, so this decoder looks subjectively a bit > better at higher quantizations by preserving more detail). > > Most of the code here was written using libdaala as a reference, > after optimizing, refactoring and shortening. As such, although very few > lines have been copied directly, the code is still licensed under the > BSD 2-clause like libdaala. Files which are completely original work are > licensed under the same license as FFmpeg. I'm not too concerned about > those, so if people want I wouldn't mind licensing them as a BSD 2-clause > like the rest of the decoder. > > The "what the hell were you thinking" Q&A section: > > Q: What about AOMedia and the IETF? > A: There's a (high) chance that the current libdaala codebase will be >used, and thus this decoder will continue tracking whatever becomes >of libdaala. > > Q: What if libdaala dies and AOMedia/IETF come up with something >completely different (unlikely). > A: This decoder will continue tracking whatever they come up with, after >salvaging all the code that can be reused. It is likely that the files >will be renamed as well as the codec description. The codec ID >(AV_CODEC_ID_DAALA) will still be used to refer to the new codec >since it's already been upstreamed. > > Signed-off-by: Rostislav Pehlivanov > --- > libavcodec/Makefile|1 + > libavcodec/allcodecs.c |1 + > libavcodec/daala.h | 78 ++ > libavcodec/daala_entropy.h | 554 > libavcodec/daala_pvq.h | 369 > libavcodec/daala_utils.h | 202 + > libavcodec/daaladec.c | 804 + > libavcodec/daaladsp.c | 2123 > > libavcodec/daaladsp.h | 78 ++ > libavcodec/daalatab.c | 1544 > libavcodec/daalatab.h | 85 ++ > 11 files changed, 5839 insertions(+) > create mode 100644 libavcodec/daala.h > create mode 100644 libavcodec/daala_entropy.h > create mode 100644 libavcodec/daala_pvq.h > create mode 100644 libavcodec/daala_utils.h > create mode 100644 libavcodec/daaladec.c > create mode 100644 libavcodec/daaladsp.c > create mode 100644 libavcodec/daaladsp.h > create mode 100644 libavcodec/daalatab.c > create mode 100644 libavcodec/daalatab.h this seems to fail building In file included from libavcodec/daaladsp.c:30:0: libavcodec/daalatab.h:48:35: error: no previous prototype for ‘daala_find_p_format’ [-Werror=missing-prototypes] cc1: some warnings being treated as errors make: *** [libavcodec/daaladsp.o] Error 1 In file included from libavcodec/daalatab.c:29:0: libavcodec/daalatab.h:48:35: error: no previous prototype for ‘daala_find_p_format’ [-Werror=missing-prototypes] cc1: some warnings being treated as errors make: *** [libavcodec/daalatab.o] Error 1 In file included from libavcodec/daaladec.c:35:0: libavcodec/daalatab.h:48:35: error: no previous prototype for ‘daala_find_p_format’ [-Werror=missing-prototypes] cc1: some warnings being treated as errors make: *** [libavcodec/daaladec.o] Error 1 make: Target `all' not remade because of errors. [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Frequently ignored answer#1 FFmpeg bugs should be sent to our bugtracker. User questions about the command line tools should be sent to the ffmpeg-user ML. And questions about how to use libav* should be sent to the libav-user ML. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] lavf/matroskadec.c: Copy QuickTime palette to st->codec->extradata
In case this patch got lost in my endless stream of previous posts, I'll repost it. Description of the patch follows: In order to make MPlayer use the correct palette when playing palettized QuickTime video in Matroska, the palette will need to be copied to st->codec->extradata in matroskadec.c. As I have explained so many times before, for some inexplicable reason, MPlayer in its current state adds *another* palette side data packet *after* the one added in matroskadec.c, using whatever is in extradata as the palette. Since this copying to st->codec->extradata is done for V_MS/VFW/FOURCC, I see no reason why it couldn't be done for V_QUICKTIME as well. I have yet to see any program use the extradata for anything else than the palette when it comes to palettized video in Matroska. I might be wrong, of course. If this patch is not acceptable, we'll have to tell the MPlayer maintainers to fix this extraneous addition of yet another palette side data packet. Unfortunately, in my experience, they seem rather slow on reacting. I would therefore be grateful if someone of the more "influential" persons here could bring this issue up in their community. Sample palettized Matroska files for testing are available at https://drive.google.com/open?id=0B3_pEBoLs0faWElmM2FnLTZYNlk. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ >From e6202f0f7caa9c82915b2fa3276b263d16d1 Mon Sep 17 00:00:00 2001 From: Mats Peterson Date: Mon, 28 Dec 2015 05:12:49 +0100 Subject: [PATCH] lavf/matroskadec.c: Copy QuickTime palette to st->codec->extradata In case this patch got lost in my endless stream of previous posts, I'll repost it. Description of the patch follows: In order to make MPlayer use the correct palette when playing palettized QuickTime video in Matroska, the palette will need to be copied to st->codec->extradata in matroskadec.c. As I have explained so many times before, for some inexplicable reason, MPlayer in its current state adds *another* palette side data packet *after* the one added in matroskadec.c, using whatever is in extradata as the palette. Since this copying to st->codec->extradata is done for V_MS/VFW/FOURCC, I see no reason why it couldn't be done for V_QUICKTIME as well. I have yet to see any program use the extradata for anything else than the palette when it comes to palettized video in Matroska. I might be wrong, of course. If this patch is not acceptable, we'll have to tell the MPlayer maintainers to fix this extraneous addition of yet another palette side data packet. Unfortunately, in my experience, they seem rather slow on reacting. I would therefore be grateful if someone of the more "influential" persons here could bring this issue up in their community. Sample palettized Matroska files for testing are available at https://drive.google.com/open?id=0B3_pEBoLs0faWElmM2FnLTZYNlk. Mats --- libavformat/matroskadec.c |9 + 1 file changed, 9 insertions(+) diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c index 9de7cfb..d788dce 100644 --- a/libavformat/matroskadec.c +++ b/libavformat/matroskadec.c @@ -1893,6 +1893,15 @@ static int matroska_parse_tracks(AVFormatContext *s) 0, NULL, NULL, NULL, NULL); if (ff_get_qtpalette(codec_id, &b, matroska->palette)) { bit_depth &= 0x1F; +/* Copy the palette to extradata. This is needed + * because MPlayer currently adds *another* palette + * side data packet *after* the one added in + * matroska_deliver_packet(), using whatever is in + * extradata as the palette. */ +if (ff_alloc_extradata(st->codec, AVPALETTE_SIZE)) +return AVERROR(ENOMEM); +memcpy(st->codec->extradata, matroska->palette, +AVPALETTE_SIZE); matroska->has_palette = 1; } } -- 1.7.10.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/srtdec: Keep exact end times
> On Dec 3, 2015, at 03:30, Eelco Lempsink wrote: > > When converting SRT to SRT (to normalize) or WebVTT the end timestamps were > modified compared to the original. > > Fixes trac 4783. > > NOTE: The FATE test 'sub-srt' fails after this patch, because the end times of > the Dialogue lines change from X:XX:XX.50 to X:XX:XX.49. I can argue that this > is semantically correct, because in the original SRT the begin and end times > are such that there is no (unintended) overlap between cues, but since the > semantics of SRT are poorly defined, I’m not sure it’s correct. This is incorrect. ASS doesn't produce overlap when 2 consecutive lines end and start on the same timestamp, since it uses `<` instead of `<=` when comparing end times. > --- > libavcodec/srtdec.c | 15 ++ > tests/ref/fate/sub-webvttenc | 66 ++-- > 2 files changed, 43 insertions(+), 38 deletions(-) > > diff --git a/libavcodec/srtdec.c b/libavcodec/srtdec.c > index 542dd35..54568ca 100644 > --- a/libavcodec/srtdec.c > +++ b/libavcodec/srtdec.c > @@ -57,7 +57,7 @@ static int srt_decode_frame(AVCodecContext *avctx, > { > AVSubtitle *sub = data; > AVBPrint buffer; > -int ts_start, ts_end, x1 = -1, y1 = -1, x2 = -1, y2 = -1; > +int ts_start, ts_duration, x1 = -1, y1 = -1, x2 = -1, y2 = -1; > int size, ret; > const uint8_t *p = av_packet_get_side_data(avpkt, > AV_PKT_DATA_SUBTITLE_POSITION, &size); > > @@ -77,12 +77,17 @@ static int srt_decode_frame(AVCodecContext *avctx, > ts_start = av_rescale_q(avpkt->pts, > avctx->time_base, > (AVRational){1,100}); > -ts_end = av_rescale_q(avpkt->pts + avpkt->duration, > -avctx->time_base, > -(AVRational){1,100}); > + > +// Floor the duration (for ASS output) > +ts_duration = avpkt->duration / 10; > + > +// Set an exact end display time to prevent the rounding for ASS messing > it up > +sub->end_display_time = av_rescale_q(avpkt->duration, > + avctx->pkt_timebase, > + (AVRational){1,1000}); Is this patch still effective if you just add this^ statement, and leave out the ts_end/ts_duration changes (to preserve the ASS rounding behavior)? > > srt_to_ass(avctx, &buffer, avpkt->data, x1, y1, x2, y2); > -ret = ff_ass_add_rect_bprint(sub, &buffer, ts_start, ts_end-ts_start); > +ret = ff_ass_add_rect_bprint(sub, &buffer, ts_start, ts_duration); > av_bprint_finalize(&buffer, NULL); > if (ret < 0) > return ret; > diff --git a/tests/ref/fate/sub-webvttenc b/tests/ref/fate/sub-webvttenc > index dbeadb0..ba567c3 100644 > --- a/tests/ref/fate/sub-webvttenc > +++ b/tests/ref/fate/sub-webvttenc > @@ -14,12 +14,12 @@ If you see this with the normal font, the player don't > (fully) support font face > 00:04.500 --> 00:04.500 > Hidden > > -00:04.501 --> 00:07.501 > +00:04.501 --> 00:07.500 > This text should be small > This text should be normal > This text should be big > > -00:07.501 --> 00:11.501 > +00:07.501 --> 00:11.500 > This should be an E with an accent: È > 日本語 > This text should be bold, italics and underline > @@ -27,7 +27,7 @@ This text should be small and green > This text should be small and red > This text should be big and brown > > -00:11.501 --> 00:14.501 > +00:11.501 --> 00:14.500 > This line should be bold > This line should be italics > This line should be underline > @@ -35,7 +35,7 @@ This line should be strikethrough > Both lines > should be underline > > -00:14.501 --> 00:17.501 > +00:14.501 --> 00:17.500 >> > It would be a good thing to > hide invalid html tags that are closed and show the text in them > @@ -43,110 +43,110 @@ hide invalid html tags that are closed and show the > text in them > Show not opened tags > < > > -00:17.501 --> 00:20.501 > +00:17.501 --> 00:20.500 > and also > hide invalid html tags with parameters that are closed and show the text in > them > but show un-closed invalid html tags > This text should be showed underlined without problems also: > 2<3,5>1,4<6 > This shouldn't be underlined > > -00:20.501 --> 00:21.501 > +00:20.501 --> 00:21.500 > This text should be in the normal position... > > -00:21.501 --> 00:22.501 > +00:21.501 --> 00:22.500 > This text should NOT be in the normal position > > -00:22.501 --> 00:24.501 > +00:22.501 --> 00:24.500 > Implementation is the same of the ASS tag > This text should be at the > top and horizontally centered > > -00:22.501 --> 00:24.501 > +00:22.501 --> 00:24.500 > This text should be at the > middle and horizontally centered > > -00:22.501 --> 00:24.501 > +00:22.501 --> 00:24.500 > This text should be at the > bottom and horizontally centered > > -00:24.501 --> 00:26.501 > +00:24.501 --> 00:26.500 > This text should be at the > top and horizontally at the left > > -00:24.501 --> 00:26.5
Re: [FFmpeg-devel] [PATCH] lavf/matroskadec.c: Copy QuickTime palette to st->codec->extradata
On 12/29/2015 03:23 AM, Mats Peterson wrote: In case this patch got lost in my endless stream of previous posts, I'll repost it. Description of the patch follows: In order to make MPlayer use the correct palette when playing palettized QuickTime video in Matroska, the palette will need to be copied to st->codec->extradata in matroskadec.c. As I have explained so many times before, for some inexplicable reason, MPlayer in its current state adds *another* palette side data packet *after* the one added in matroskadec.c, using whatever is in extradata as the palette. Since this copying to st->codec->extradata is done for V_MS/VFW/FOURCC, I see no reason why it couldn't be done for V_QUICKTIME as well. I have yet to see any program use the extradata for anything else than the palette when it comes to palettized video in Matroska. I might be wrong, of course. If this patch is not acceptable, we'll have to tell the MPlayer maintainers to fix this extraneous addition of yet another palette side data packet. Unfortunately, in my experience, they seem rather slow on reacting. I would therefore be grateful if someone of the more "influential" persons here could bring this issue up in their community. Sample palettized Matroska files for testing are available at https://drive.google.com/open?id=0B3_pEBoLs0faWElmM2FnLTZYNlk. Mats ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Well, what do you know. It seems the issue has already been fixed in MPlayer. I just recompiled it with the latest FFmpeg. Black magic. Forget this patch then. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] Doxy documentation for qtpalette.c vanished?
Excuse me, but where did the doxy documentation for ff_get_qtpalette() in qtpalette.c go? Michael? Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Doxy documentation for qtpalette.c vanished?
On Tue, Dec 29, 2015 at 5:26 AM, Mats Peterson wrote: > Excuse me, but where did the doxy documentation for ff_get_qtpalette() in > qtpalette.c go? Michael? > > Mats > > -- > Mats Peterson > http://matsp888.no-ip.org/~mats/ > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel A quick git log --follow qtpalette.c will tell you: $ git log --follow qtpalette.c commit ca9e3cb3ce7a6207570500e003ec2ed44836921e Author: Michael Niedermayer Date: Mon Dec 28 03:18:20 2015 +0100 avformat/qtpalette: Move ff_get_qtpalette() doxy to header Signed-off-by: Michael Niedermayer Regards, Stephan ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Doxy documentation for qtpalette.c vanished?
On 12/29/2015 05:26 AM, Mats Peterson wrote: Excuse me, but where did the doxy documentation for ff_get_qtpalette() in qtpalette.c go? Michael? Mats Ah, it's in qtpalette.h. Sorry. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] lavf/qtpalette: Treat 1-bit video as palettized
This patch for qtpalette.c and qtpalette.h adds 1-bit video to the "palettized video" category, since if the video sample description contains a palette, the two colors in the palette can be any color, not necessarily black & white. Unfortunately, I've noticed that the qtrle (QuickTime Animation) decoder blindly assumes that 1-bit video is black & white. I don't have enough knowledge about the decoder to fix this, though. Below is a link to a sample 1-bit QuickTime Animation clip of a rotating earth that uses blueish colors, and they will be correctly rendered in QuickTime, but not in FFmpeg (which will use black & white). https://drive.google.com/open?id=0B3_pEBoLs0faUlItWm9KaGJSTEE Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ >From 172d0252f8c4b9ee8032cf43d43322ced547d7a8 Mon Sep 17 00:00:00 2001 From: Mats Peterson Date: Tue, 29 Dec 2015 06:22:35 +0100 Subject: [PATCH] lavf/qtpalette: Treat 1-bit video as palettized This patch for qtpalette.c and qtpalette.h adds 1-bit video to the "palettized video" category, since if the video sample description contains a palette, the two colors in the palette can be any color, not necessarily black & white. Unfortunately, I've noticed that the qtrle (QuickTime Animation) decoder blindly assumes that 1-bit video is black & white. I don't have enough knowledge about the decoder to fix this, though. Below is a link to a sample 1-bit QuickTime Animation clip of a rotating earth that uses blueish colors, and they will be correctly rendered in QuickTime, but not in FFmpeg (which will use black & white). https://drive.google.com/open?id=0B3_pEBoLs0faUlItWm9KaGJSTEE Mats --- libavformat/qtpalette.c |4 ++-- libavformat/qtpalette.h |5 + 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/libavformat/qtpalette.c b/libavformat/qtpalette.c index 770df56..4db8d2b 100644 --- a/libavformat/qtpalette.c +++ b/libavformat/qtpalette.c @@ -46,8 +46,8 @@ int ff_get_qtpalette(int codec_id, AVIOContext *pb, uint32_t *palette) if (greyscale && codec_id == AV_CODEC_ID_CINEPAK) return 0; -/* If the depth is 2, 4, or 8 bpp, file is palettized. */ -if ((bit_depth == 2 || bit_depth == 4 || bit_depth == 8)) { +/* If the depth is 1, 2, 4, or 8 bpp, file is palettized. */ +if ((bit_depth == 1 || bit_depth == 2 || bit_depth == 4 || bit_depth == 8)) { int color_count, color_start, color_end; uint32_t a, r, g, b; diff --git a/libavformat/qtpalette.h b/libavformat/qtpalette.h index da83421..b2bd360 100644 --- a/libavformat/qtpalette.h +++ b/libavformat/qtpalette.h @@ -26,6 +26,11 @@ #include #include "avformat.h" +static const uint8_t ff_qt_default_palette_2[2 * 3] = { + 0xFF, 0xFF, 0xFF, + 0x00, 0x00, 0x00 +}; + static const uint8_t ff_qt_default_palette_4[4 * 3] = { 0x93, 0x65, 0x5E, 0xFF, 0xFF, 0xFF, -- 1.7.10.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avformat/concatdec: get bit_rate from internal format
--- libavformat/concatdec.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c index d21805f..4c7217c 100644 --- a/libavformat/concatdec.c +++ b/libavformat/concatdec.c @@ -310,6 +310,7 @@ static int open_file(AVFormatContext *avf, unsigned fileno) avformat_close_input(&cat->avf); return ret; } +avf->bit_rate = cat->avf->bit_rate; cat->cur_file = file; if (file->start_time == AV_NOPTS_VALUE) file->start_time = !fileno ? 0 : -- 2.5.3 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavfi/af_anequalizer: remove cabs, cexp dependencies
On 29 December 2015 at 11:51, Ganesh Ajjanagadde wrote: > On Mon, Dec 28, 2015 at 8:22 AM, Paul B Mahol wrote: > > On 12/27/15, Ganesh Ajjanagadde wrote: > >> Replaces by real arithmetic. Tested the validity of these > transformations > >> separately. > >> > >> Signed-off-by: Ganesh Ajjanagadde > >> --- > >> configure| 1 - > >> libavfilter/af_anequalizer.c | 25 + > >> 2 files changed, 17 insertions(+), 9 deletions(-) > >> > > > > If those are mathematically equivalent ok. > > mathematically equivalent, tested offline, numerical differences of > the order of ~ 1e-15 or so, which is perfectly fine IMHO for double > precision especially since it is a priori not clear which is more > precise. Pushed, thanks. Wouldn't it also be a good idea to remove the COMPLEX_FUNCS list from configure as now neither cabs or cexp are used so configure can be simplified by removing the checks for them. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH v2] lavf/qtpalette: Treat 1-bit video as palettized
I forgot some stuff. Patch description follows: This patch for qtpalette.c and qtpalette.h adds 1-bit video to the "palettized video" category, since if the video sample description contains a palette, the two colors in the palette can be any color, not necessarily black & white. Unfortunately, I've noticed that the qtrle (QuickTime Animation) decoder blindly assumes that 1-bit video is black & white. I don't have enough knowledge about the decoder to fix this, though. Below is a link to a sample 1-bit QuickTime Animation clip of a rotating earth that uses blueish colors, and they will be correctly rendered in QuickTime, but not in FFmpeg (which will use black & white). https://drive.google.com/open?id=0B3_pEBoLs0faUlItWm9KaGJSTEE Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ >From 64dbe9e113d5ba3fc03951711ffb6c51b008 Mon Sep 17 00:00:00 2001 From: Mats Peterson Date: Tue, 29 Dec 2015 07:35:00 +0100 Subject: [PATCH v2] lavf/qtpalette: Treat 1-bit video as palettized I forgot some stuff. Patch description follows: This patch for qtpalette.c and qtpalette.h adds 1-bit video to the "palettized video" category, since if the video sample description contains a palette, the two colors in the palette can be any color, not necessarily black & white. Unfortunately, I've noticed that the qtrle (QuickTime Animation) decoder blindly assumes that 1-bit video is black & white. I don't have enough knowledge about the decoder to fix this, though. Below is a link to a sample 1-bit QuickTime Animation clip of a rotating earth that uses blueish colors, and they will be correctly rendered in QuickTime, but not in FFmpeg (which will use black & white). https://drive.google.com/open?id=0B3_pEBoLs0faUlItWm9KaGJSTEE Mats --- libavformat/qtpalette.c |8 +--- libavformat/qtpalette.h |5 + 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/libavformat/qtpalette.c b/libavformat/qtpalette.c index 770df56..6544a55 100644 --- a/libavformat/qtpalette.c +++ b/libavformat/qtpalette.c @@ -46,8 +46,8 @@ int ff_get_qtpalette(int codec_id, AVIOContext *pb, uint32_t *palette) if (greyscale && codec_id == AV_CODEC_ID_CINEPAK) return 0; -/* If the depth is 2, 4, or 8 bpp, file is palettized. */ -if ((bit_depth == 2 || bit_depth == 4 || bit_depth == 8)) { +/* If the depth is 1, 2, 4, or 8 bpp, file is palettized. */ +if ((bit_depth == 1 || bit_depth == 2 || bit_depth == 4 || bit_depth == 8)) { int color_count, color_start, color_end; uint32_t a, r, g, b; @@ -70,7 +70,9 @@ int ff_get_qtpalette(int codec_id, AVIOContext *pb, uint32_t *palette) * color table */ const uint8_t *color_table; color_count = 1 << bit_depth; -if (bit_depth == 2) +if (bit_depth == 1) +color_table = ff_qt_default_palette_2; +else if (bit_depth == 2) color_table = ff_qt_default_palette_4; else if (bit_depth == 4) color_table = ff_qt_default_palette_16; diff --git a/libavformat/qtpalette.h b/libavformat/qtpalette.h index da83421..b2bd360 100644 --- a/libavformat/qtpalette.h +++ b/libavformat/qtpalette.h @@ -26,6 +26,11 @@ #include #include "avformat.h" +static const uint8_t ff_qt_default_palette_2[2 * 3] = { + 0xFF, 0xFF, 0xFF, + 0x00, 0x00, 0x00 +}; + static const uint8_t ff_qt_default_palette_4[4 * 3] = { 0x93, 0x65, 0x5E, 0xFF, 0xFF, 0xFF, -- 1.7.10.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avcodec: Use get_ue_golomb_long() when needed
get_ue_golomb() cannot decode values larger than 8190 (the maximum value that can be golomb encoded in 25 bits) and produces the error "Invalid UE golomb code" if a larger value is encountered. Use get_ue_golomb_long() instead (which supports 63 bits, up to 4294967294) when valid h264/hevc values can exceed 8190. This updates decoding of the following values: (maximum) first_mb_in_slice36863* for level 5.2 abs_diff_pic_num_minus1 131071 difference_of_pic_nums_minus1 131071 idr_pic_id 65535 recovery_frame_cnt 65535 frame_packing_arrangement_id4294967294 frame_packing_arrangement_repetition_period 16384 display_orientation_repetition_period16384 An alternative would be to modify get_ue_golomb() to handle encoded values of up to 49 bits as was done for get_se_golomb() in a92816c. In that case get_ue_golomb() could continue to be used for all of these except frame_packing_arrangement_id. --- libavcodec/golomb.h | 2 +- libavcodec/h264.c| 2 +- libavcodec/h264_parser.c | 6 +++--- libavcodec/h264_refs.c | 4 ++-- libavcodec/h264_sei.c| 10 +- libavcodec/h264_slice.c | 2 +- libavcodec/hevc_sei.c| 2 +- 7 files changed, 14 insertions(+), 14 deletions(-) diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h index 5136a04..d4df0b3 100644 --- a/libavcodec/golomb.h +++ b/libavcodec/golomb.h @@ -48,7 +48,7 @@ extern const int8_t ff_interleaved_se_golomb_vlc_code[256]; extern const uint8_t ff_interleaved_dirac_golomb_vlc_code[256]; /** - * read unsigned exp golomb code. + * Read an unsigned Exp-Golomb code in the range 0 to 8190. */ static inline int get_ue_golomb(GetBitContext *gb) { diff --git a/libavcodec/h264.c b/libavcodec/h264.c index 089a86f..139011b 100644 --- a/libavcodec/h264.c +++ b/libavcodec/h264.c @@ -1365,7 +1365,7 @@ static int get_last_needed_nal(H264Context *h, const uint8_t *buf, int buf_size) case NAL_IDR_SLICE: case NAL_SLICE: init_get_bits(&gb, ptr, bit_length); -if (!get_ue_golomb(&gb) || +if (!get_ue_golomb_long(&gb) || // first_mb_in_slice !first_slice || first_slice != h->nal_unit_type) nals_needed = nal_index; diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c index 19d1aa3..12d6397 100644 --- a/libavcodec/h264_parser.c +++ b/libavcodec/h264_parser.c @@ -152,7 +152,7 @@ static int scan_mmco_reset(AVCodecParserContext *s) unsigned int reordering_of_pic_nums_idc = get_ue_golomb_31(&sl->gb); if (reordering_of_pic_nums_idc < 3) -get_ue_golomb(&sl->gb); +get_ue_golomb_long(&sl->gb); else if (reordering_of_pic_nums_idc > 3) { av_log(h->avctx, AV_LOG_ERROR, "illegal reordering_of_pic_nums_idc %d\n", @@ -191,7 +191,7 @@ static int scan_mmco_reset(AVCodecParserContext *s) return 1; if (opcode == MMCO_SHORT2UNUSED || opcode == MMCO_SHORT2LONG) -get_ue_golomb(&sl->gb); +get_ue_golomb_long(&sl->gb); // difference_of_pic_nums_minus1 if (opcode == MMCO_SHORT2LONG || opcode == MMCO_LONG2UNUSED || opcode == MMCO_LONG || opcode == MMCO_SET_MAX_LONG) get_ue_golomb_31(&sl->gb); @@ -373,7 +373,7 @@ static inline int parse_nal_units(AVCodecParserContext *s, } if (h->nal_unit_type == NAL_IDR_SLICE) -get_ue_golomb(&sl->gb); /* idr_pic_id */ +get_ue_golomb_long(&sl->gb); /* idr_pic_id */ if (h->sps.poc_type == 0) { h->poc_lsb = get_bits(&sl->gb, h->sps.log2_max_poc_lsb); diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c index fbdcbd6..52fedc1 100644 --- a/libavcodec/h264_refs.c +++ b/libavcodec/h264_refs.c @@ -266,7 +266,7 @@ int ff_h264_decode_ref_pic_list_reordering(H264Context *h, H264SliceContext *sl) switch (modification_of_pic_nums_idc) { case 0: case 1: { -const unsigned int abs_diff_pic_num = get_ue_golomb(&sl->gb) + 1; +const unsigned int abs_diff_pic_num = get_ue_golomb_long(&sl->gb) + 1; int frame_num; if (abs_diff_pic_num > h->max_pic_num) { @@ -843,7 +843,7 @@ int ff_h264_decode_ref_pic_marking(H264Context *h, GetBitContext *gb, mmco[i].opcode = opcode; if (opcode == MMCO_SHORT2UNUSED || opcode == MMCO_SHORT2LONG) { mmco[i].short_pic_num = -(h->curr_pic_num - get_ue_golomb(gb) - 1) & +(h->curr_pic_num - get_ue_golomb_long(gb)