| Message ID | 20220610231045.9760-2-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/3] avcodec/fmvc: Move frame allocation to a later stage | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
Have you actually tested this "change" ?
On Sat, Jun 11, 2022 at 10:47:57AM +0200, Paul B Mahol wrote:
> Have you actually tested this "change" ?
On every file i found
6-methyl-5-hepten-2-one-CC-db_small.avi
fmvcVirtualDub_small.avi
skrzyzowanie4.avi
fmvc-poc.avi
are there any other files i should test it on ?
thx
[...]
On Sat, Jun 11, 2022 at 4:55 PM Michael Niedermayer <michael@niedermayer.cc> wrote: > On Sat, Jun 11, 2022 at 10:47:57AM +0200, Paul B Mahol wrote: > > Have you actually tested this "change" ? > > On every file i found > 6-methyl-5-hepten-2-one-CC-db_small.avi > fmvcVirtualDub_small.avi > skrzyzowanie4.avi > fmvc-poc.avi > > are there any other files i should test it on ? > Yes, the ones where stride != width. > > thx > > [...] > > -- > 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 > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
Quoting Paul B Mahol (2022-06-13 10:04:04) > On Sat, Jun 11, 2022 at 4:55 PM Michael Niedermayer <michael@niedermayer.cc> > wrote: > > > On Sat, Jun 11, 2022 at 10:47:57AM +0200, Paul B Mahol wrote: > > > Have you actually tested this "change" ? > > > > On every file i found > > 6-methyl-5-hepten-2-one-CC-db_small.avi > > fmvcVirtualDub_small.avi > > skrzyzowanie4.avi > > fmvc-poc.avi > > > > are there any other files i should test it on ? > > > > Yes, the ones where stride != width. Give examples of such files then. And add more tests. You really should try to be more helpful if you care about this code working.
On Mon, Jun 13, 2022 at 11:10 AM Anton Khirnov <anton@khirnov.net> wrote: > Quoting Paul B Mahol (2022-06-13 10:04:04) > > On Sat, Jun 11, 2022 at 4:55 PM Michael Niedermayer < > michael@niedermayer.cc> > > wrote: > > > > > On Sat, Jun 11, 2022 at 10:47:57AM +0200, Paul B Mahol wrote: > > > > Have you actually tested this "change" ? > > > > > > On every file i found > > > 6-methyl-5-hepten-2-one-CC-db_small.avi > > > fmvcVirtualDub_small.avi > > > skrzyzowanie4.avi > > > fmvc-poc.avi > > > > > > are there any other files i should test it on ? > > > > > > > Yes, the ones where stride != width. > > Give examples of such files then. And add more tests. > > You really should try to be more helpful if you care about this code > working. Code works perfectly from start. There are always attempts to break it. Your attempts to belittle my work are futile. > > -- > Anton Khirnov > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
Quoting Paul B Mahol (2022-06-13 11:34:44) > On Mon, Jun 13, 2022 at 11:10 AM Anton Khirnov <anton@khirnov.net> wrote: > > > Quoting Paul B Mahol (2022-06-13 10:04:04) > > > On Sat, Jun 11, 2022 at 4:55 PM Michael Niedermayer < > > michael@niedermayer.cc> > > > wrote: > > > > > > > On Sat, Jun 11, 2022 at 10:47:57AM +0200, Paul B Mahol wrote: > > > > > Have you actually tested this "change" ? > > > > > > > > On every file i found > > > > 6-methyl-5-hepten-2-one-CC-db_small.avi > > > > fmvcVirtualDub_small.avi > > > > skrzyzowanie4.avi > > > > fmvc-poc.avi > > > > > > > > are there any other files i should test it on ? > > > > > > > > > > Yes, the ones where stride != width. > > > > Give examples of such files then. And add more tests. > > > > You really should try to be more helpful if you care about this code > > working. > > > Code works perfectly from start. There are always attempts to break it. > Your attempts to belittle my work are futile. Perfect code should live in an external repository that is locked against modification. The ffmpeg repository is only for imperfect code that evolves with time, and so requires changes.
On Mon, Jun 13, 2022 at 11:48 AM Anton Khirnov <anton@khirnov.net> wrote: > Quoting Paul B Mahol (2022-06-13 11:34:44) > > On Mon, Jun 13, 2022 at 11:10 AM Anton Khirnov <anton@khirnov.net> > wrote: > > > > > Quoting Paul B Mahol (2022-06-13 10:04:04) > > > > On Sat, Jun 11, 2022 at 4:55 PM Michael Niedermayer < > > > michael@niedermayer.cc> > > > > wrote: > > > > > > > > > On Sat, Jun 11, 2022 at 10:47:57AM +0200, Paul B Mahol wrote: > > > > > > Have you actually tested this "change" ? > > > > > > > > > > On every file i found > > > > > 6-methyl-5-hepten-2-one-CC-db_small.avi > > > > > fmvcVirtualDub_small.avi > > > > > skrzyzowanie4.avi > > > > > fmvc-poc.avi > > > > > > > > > > are there any other files i should test it on ? > > > > > > > > > > > > > Yes, the ones where stride != width. > > > > > > Give examples of such files then. And add more tests. > > > > > > You really should try to be more helpful if you care about this code > > > working. > > > > > > Code works perfectly from start. There are always attempts to break it. > > Your attempts to belittle my work are futile. > > Perfect code should live in an external repository that is locked > against modification. > > The ffmpeg repository is only for imperfect code that evolves with time, > and so requires changes. > > I dunno what Michael attempts to fix. Decoder works fine with valid files. I doubt that encoder would encode random bytes or padding into valid file bitstream. > -- > Anton Khirnov > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
On Mon, Jun 13, 2022 at 12:10:44PM +0200, Paul B Mahol wrote: > On Mon, Jun 13, 2022 at 11:48 AM Anton Khirnov <anton@khirnov.net> wrote: > > > Quoting Paul B Mahol (2022-06-13 11:34:44) > > > On Mon, Jun 13, 2022 at 11:10 AM Anton Khirnov <anton@khirnov.net> > > wrote: > > > > > > > Quoting Paul B Mahol (2022-06-13 10:04:04) > > > > > On Sat, Jun 11, 2022 at 4:55 PM Michael Niedermayer < > > > > michael@niedermayer.cc> > > > > > wrote: > > > > > > > > > > > On Sat, Jun 11, 2022 at 10:47:57AM +0200, Paul B Mahol wrote: > > > > > > > Have you actually tested this "change" ? > > > > > > > > > > > > On every file i found > > > > > > 6-methyl-5-hepten-2-one-CC-db_small.avi > > > > > > fmvcVirtualDub_small.avi > > > > > > skrzyzowanie4.avi > > > > > > fmvc-poc.avi > > > > > > > > > > > > are there any other files i should test it on ? > > > > > > > > > > > > > > > > Yes, the ones where stride != width. > > > > > > > > Give examples of such files then. And add more tests. > > > > > > > > You really should try to be more helpful if you care about this code > > > > working. > > > > > > > > > Code works perfectly from start. There are always attempts to break it. > > > Your attempts to belittle my work are futile. > > > > Perfect code should live in an external repository that is locked > > against modification. > > > > The ffmpeg repository is only for imperfect code that evolves with time, > > and so requires changes. > > > > > I dunno what Michael attempts to fix. Decoder works fine with valid files. > I doubt that encoder would encode random bytes or padding into valid file > bitstream. the stride*4 / width*4 change was because of 2 things. first with AV_PIX_FMT_BGR24 the data stored is not width*4 stride is in units of 4 bytes for some reason, so stride*4 fixes this The 2nd issue is that the code addresses it by "s->stride * 4" so the buffer allocation should be stride*4 if we belive the other code is correct src = s->buffer; ... for (y = 0; y < avctx->height; y++) { ... src += s->stride * 4; width*4 works because its bigger than stride*4 for BGR24 which is what all samples i have use. also ssrc = s->buffer; ... for (y = 0; y < avctx->height; y++) { ... ssrc += s->stride * 4; and dst = (uint32_t *)s->buffer; for (block = 0, y = 0; y < s->yb; y++) { int block_h = s->blocks[block].h; uint32_t *rect = dst; for (x = 0; x < s->xb; x++) { int block_w = s->blocks[block].w; uint32_t *row = dst; block_h = s->blocks[block].h; if (s->blocks[block].xor) { for (k = 0; k < block_h; k++) { uint32_t *column = dst; for (l = 0; l < block_w; l++) *dst++ ^= *src++; dst = &column[s->stride]; } } dst = &row[block_w]; ++block; } dst = &rect[block_h * s->stride]; } Again, if you have fmvc files with more odd widths or other pixel formats these would be very welcome. I can just say the code as is in git is wrong and the buffer size as is in git is wrong. I noticed this when i added a check to see if the buffer is only partly filled and realized its always partly filled even when the whole image is actually touched thx [...]
On Mon, Jun 13, 2022 at 09:13:19PM +0200, Michael Niedermayer wrote: > On Mon, Jun 13, 2022 at 12:10:44PM +0200, Paul B Mahol wrote: > > On Mon, Jun 13, 2022 at 11:48 AM Anton Khirnov <anton@khirnov.net> wrote: > > > > > Quoting Paul B Mahol (2022-06-13 11:34:44) > > > > On Mon, Jun 13, 2022 at 11:10 AM Anton Khirnov <anton@khirnov.net> > > > wrote: > > > > > > > > > Quoting Paul B Mahol (2022-06-13 10:04:04) > > > > > > On Sat, Jun 11, 2022 at 4:55 PM Michael Niedermayer < > > > > > michael@niedermayer.cc> > > > > > > wrote: > > > > > > > > > > > > > On Sat, Jun 11, 2022 at 10:47:57AM +0200, Paul B Mahol wrote: > > > > > > > > Have you actually tested this "change" ? > > > > > > > > > > > > > > On every file i found > > > > > > > 6-methyl-5-hepten-2-one-CC-db_small.avi > > > > > > > fmvcVirtualDub_small.avi > > > > > > > skrzyzowanie4.avi > > > > > > > fmvc-poc.avi > > > > > > > > > > > > > > are there any other files i should test it on ? > > > > > > > > > > > > > > > > > > > Yes, the ones where stride != width. > > > > > > > > > > Give examples of such files then. And add more tests. > > > > > > > > > > You really should try to be more helpful if you care about this code > > > > > working. > > > > > > > > > > > > Code works perfectly from start. There are always attempts to break it. > > > > Your attempts to belittle my work are futile. > > > > > > Perfect code should live in an external repository that is locked > > > against modification. > > > > > > The ffmpeg repository is only for imperfect code that evolves with time, > > > and so requires changes. > > > > > > > > I dunno what Michael attempts to fix. Decoder works fine with valid files. > > I doubt that encoder would encode random bytes or padding into valid file > > bitstream. > > the stride*4 / width*4 change was because of 2 things. > first with AV_PIX_FMT_BGR24 the data stored is not width*4 > > stride is in units of 4 bytes for some reason, so stride*4 > fixes this > The 2nd issue is that the code addresses it by "s->stride * 4" > so the buffer allocation should be stride*4 if we belive the > other code is correct > > src = s->buffer; > ... > for (y = 0; y < avctx->height; y++) { > ... > src += s->stride * 4; > > width*4 works because its bigger than stride*4 for BGR24 which is what all > samples i have use. > > also > ssrc = s->buffer; > ... > for (y = 0; y < avctx->height; y++) { > ... > ssrc += s->stride * 4; > and > dst = (uint32_t *)s->buffer; > > for (block = 0, y = 0; y < s->yb; y++) { > int block_h = s->blocks[block].h; > uint32_t *rect = dst; > > for (x = 0; x < s->xb; x++) { > int block_w = s->blocks[block].w; > uint32_t *row = dst; > > block_h = s->blocks[block].h; > if (s->blocks[block].xor) { > for (k = 0; k < block_h; k++) { > uint32_t *column = dst; > for (l = 0; l < block_w; l++) > *dst++ ^= *src++; > dst = &column[s->stride]; > } > } > dst = &row[block_w]; > ++block; > } > dst = &rect[block_h * s->stride]; > } > > Again, if you have fmvc files with more odd widths or other pixel formats > these would be very welcome. I can just say the code as is in git is wrong > and the buffer size as is in git is wrong. I noticed this when i added > a check to see if the buffer is only partly filled and realized its > always partly filled even when the whole image is actually touched If there are no objections aka noone sees a bug in this then id like to apply this thx [...]
On Fri, Sep 2, 2022 at 6:32 PM Michael Niedermayer <michael@niedermayer.cc> wrote: > On Mon, Jun 13, 2022 at 09:13:19PM +0200, Michael Niedermayer wrote: > > On Mon, Jun 13, 2022 at 12:10:44PM +0200, Paul B Mahol wrote: > > > On Mon, Jun 13, 2022 at 11:48 AM Anton Khirnov <anton@khirnov.net> > wrote: > > > > > > > Quoting Paul B Mahol (2022-06-13 11:34:44) > > > > > On Mon, Jun 13, 2022 at 11:10 AM Anton Khirnov <anton@khirnov.net> > > > > wrote: > > > > > > > > > > > Quoting Paul B Mahol (2022-06-13 10:04:04) > > > > > > > On Sat, Jun 11, 2022 at 4:55 PM Michael Niedermayer < > > > > > > michael@niedermayer.cc> > > > > > > > wrote: > > > > > > > > > > > > > > > On Sat, Jun 11, 2022 at 10:47:57AM +0200, Paul B Mahol wrote: > > > > > > > > > Have you actually tested this "change" ? > > > > > > > > > > > > > > > > On every file i found > > > > > > > > 6-methyl-5-hepten-2-one-CC-db_small.avi > > > > > > > > fmvcVirtualDub_small.avi > > > > > > > > skrzyzowanie4.avi > > > > > > > > fmvc-poc.avi > > > > > > > > > > > > > > > > are there any other files i should test it on ? > > > > > > > > > > > > > > > > > > > > > > Yes, the ones where stride != width. > > > > > > > > > > > > Give examples of such files then. And add more tests. > > > > > > > > > > > > You really should try to be more helpful if you care about this > code > > > > > > working. > > > > > > > > > > > > > > > Code works perfectly from start. There are always attempts to > break it. > > > > > Your attempts to belittle my work are futile. > > > > > > > > Perfect code should live in an external repository that is locked > > > > against modification. > > > > > > > > The ffmpeg repository is only for imperfect code that evolves with > time, > > > > and so requires changes. > > > > > > > > > > > I dunno what Michael attempts to fix. Decoder works fine with valid > files. > > > I doubt that encoder would encode random bytes or padding into valid > file > > > bitstream. > > > > the stride*4 / width*4 change was because of 2 things. > > first with AV_PIX_FMT_BGR24 the data stored is not width*4 > > > > stride is in units of 4 bytes for some reason, so stride*4 > > fixes this > > The 2nd issue is that the code addresses it by "s->stride * 4" > > so the buffer allocation should be stride*4 if we belive the > > other code is correct > > > > src = s->buffer; > > ... > > for (y = 0; y < avctx->height; y++) { > > ... > > src += s->stride * 4; > > > > width*4 works because its bigger than stride*4 for BGR24 which is what > all > > samples i have use. > > > > also > > ssrc = s->buffer; > > ... > > for (y = 0; y < avctx->height; y++) { > > ... > > ssrc += s->stride * 4; > > and > > dst = (uint32_t *)s->buffer; > > > > for (block = 0, y = 0; y < s->yb; y++) { > > int block_h = s->blocks[block].h; > > uint32_t *rect = dst; > > > > for (x = 0; x < s->xb; x++) { > > int block_w = s->blocks[block].w; > > uint32_t *row = dst; > > > > block_h = s->blocks[block].h; > > if (s->blocks[block].xor) { > > for (k = 0; k < block_h; k++) { > > uint32_t *column = dst; > > for (l = 0; l < block_w; l++) > > *dst++ ^= *src++; > > dst = &column[s->stride]; > > } > > } > > dst = &row[block_w]; > > ++block; > > } > > dst = &rect[block_h * s->stride]; > > } > > > > Again, if you have fmvc files with more odd widths or other pixel formats > > these would be very welcome. I can just say the code as is in git is > wrong > > and the buffer size as is in git is wrong. I noticed this when i added > > a check to see if the buffer is only partly filled and realized its > > always partly filled even when the whole image is actually touched > > If there are no objections aka noone sees a bug in this then id like > to apply this > Since when are partially filled buffers are bad thing? > > thx > > [...] > > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > What does censorship reveal? It reveals fear. -- Julian Assange > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
On Fri, Sep 02, 2022 at 06:48:57PM +0200, Paul B Mahol wrote: > On Fri, Sep 2, 2022 at 6:32 PM Michael Niedermayer <michael@niedermayer.cc> > wrote: > > > On Mon, Jun 13, 2022 at 09:13:19PM +0200, Michael Niedermayer wrote: > > > On Mon, Jun 13, 2022 at 12:10:44PM +0200, Paul B Mahol wrote: > > > > On Mon, Jun 13, 2022 at 11:48 AM Anton Khirnov <anton@khirnov.net> > > wrote: > > > > > > > > > Quoting Paul B Mahol (2022-06-13 11:34:44) > > > > > > On Mon, Jun 13, 2022 at 11:10 AM Anton Khirnov <anton@khirnov.net> > > > > > wrote: > > > > > > > > > > > > > Quoting Paul B Mahol (2022-06-13 10:04:04) > > > > > > > > On Sat, Jun 11, 2022 at 4:55 PM Michael Niedermayer < > > > > > > > michael@niedermayer.cc> > > > > > > > > wrote: > > > > > > > > > > > > > > > > > On Sat, Jun 11, 2022 at 10:47:57AM +0200, Paul B Mahol wrote: > > > > > > > > > > Have you actually tested this "change" ? > > > > > > > > > > > > > > > > > > On every file i found > > > > > > > > > 6-methyl-5-hepten-2-one-CC-db_small.avi > > > > > > > > > fmvcVirtualDub_small.avi > > > > > > > > > skrzyzowanie4.avi > > > > > > > > > fmvc-poc.avi > > > > > > > > > > > > > > > > > > are there any other files i should test it on ? > > > > > > > > > > > > > > > > > > > > > > > > > Yes, the ones where stride != width. > > > > > > > > > > > > > > Give examples of such files then. And add more tests. > > > > > > > > > > > > > > You really should try to be more helpful if you care about this > > code > > > > > > > working. > > > > > > > > > > > > > > > > > > Code works perfectly from start. There are always attempts to > > break it. > > > > > > Your attempts to belittle my work are futile. > > > > > > > > > > Perfect code should live in an external repository that is locked > > > > > against modification. > > > > > > > > > > The ffmpeg repository is only for imperfect code that evolves with > > time, > > > > > and so requires changes. > > > > > > > > > > > > > > I dunno what Michael attempts to fix. Decoder works fine with valid > > files. > > > > I doubt that encoder would encode random bytes or padding into valid > > file > > > > bitstream. > > > > > > the stride*4 / width*4 change was because of 2 things. > > > first with AV_PIX_FMT_BGR24 the data stored is not width*4 > > > > > > stride is in units of 4 bytes for some reason, so stride*4 > > > fixes this > > > The 2nd issue is that the code addresses it by "s->stride * 4" > > > so the buffer allocation should be stride*4 if we belive the > > > other code is correct > > > > > > src = s->buffer; > > > ... > > > for (y = 0; y < avctx->height; y++) { > > > ... > > > src += s->stride * 4; > > > > > > width*4 works because its bigger than stride*4 for BGR24 which is what > > all > > > samples i have use. > > > > > > also > > > ssrc = s->buffer; > > > ... > > > for (y = 0; y < avctx->height; y++) { > > > ... > > > ssrc += s->stride * 4; > > > and > > > dst = (uint32_t *)s->buffer; > > > > > > for (block = 0, y = 0; y < s->yb; y++) { > > > int block_h = s->blocks[block].h; > > > uint32_t *rect = dst; > > > > > > for (x = 0; x < s->xb; x++) { > > > int block_w = s->blocks[block].w; > > > uint32_t *row = dst; > > > > > > block_h = s->blocks[block].h; > > > if (s->blocks[block].xor) { > > > for (k = 0; k < block_h; k++) { > > > uint32_t *column = dst; > > > for (l = 0; l < block_w; l++) > > > *dst++ ^= *src++; > > > dst = &column[s->stride]; > > > } > > > } > > > dst = &row[block_w]; > > > ++block; > > > } > > > dst = &rect[block_h * s->stride]; > > > } > > > > > > Again, if you have fmvc files with more odd widths or other pixel formats > > > these would be very welcome. I can just say the code as is in git is > > wrong > > > and the buffer size as is in git is wrong. I noticed this when i added > > > a check to see if the buffer is only partly filled and realized its > > > always partly filled even when the whole image is actually touched > > > > If there are no objections aka noone sees a bug in this then id like > > to apply this > > > > Since when are partially filled buffers are bad thing? - waste of memory - breaks subsequent patch - width and stride relate this way: s->stride = (avctx->width * avctx->bits_per_coded_sample + 31) / 32; is width always bigger or equal ? If not we might be accessing outside the array because access uses stride, allocation width Or one line awnser Since when is it a good thing to mismatch allocation and access? thx [...]
On Sun, Sep 04, 2022 at 11:42:49PM +0200, Michael Niedermayer wrote: > On Fri, Sep 02, 2022 at 06:48:57PM +0200, Paul B Mahol wrote: > > On Fri, Sep 2, 2022 at 6:32 PM Michael Niedermayer <michael@niedermayer.cc> > > wrote: > > > > > On Mon, Jun 13, 2022 at 09:13:19PM +0200, Michael Niedermayer wrote: > > > > On Mon, Jun 13, 2022 at 12:10:44PM +0200, Paul B Mahol wrote: > > > > > On Mon, Jun 13, 2022 at 11:48 AM Anton Khirnov <anton@khirnov.net> > > > wrote: > > > > > > > > > > > Quoting Paul B Mahol (2022-06-13 11:34:44) > > > > > > > On Mon, Jun 13, 2022 at 11:10 AM Anton Khirnov <anton@khirnov.net> > > > > > > wrote: > > > > > > > > > > > > > > > Quoting Paul B Mahol (2022-06-13 10:04:04) > > > > > > > > > On Sat, Jun 11, 2022 at 4:55 PM Michael Niedermayer < > > > > > > > > michael@niedermayer.cc> > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > On Sat, Jun 11, 2022 at 10:47:57AM +0200, Paul B Mahol wrote: > > > > > > > > > > > Have you actually tested this "change" ? > > > > > > > > > > > > > > > > > > > > On every file i found > > > > > > > > > > 6-methyl-5-hepten-2-one-CC-db_small.avi > > > > > > > > > > fmvcVirtualDub_small.avi > > > > > > > > > > skrzyzowanie4.avi > > > > > > > > > > fmvc-poc.avi > > > > > > > > > > > > > > > > > > > > are there any other files i should test it on ? > > > > > > > > > > > > > > > > > > > > > > > > > > > > Yes, the ones where stride != width. > > > > > > > > > > > > > > > > Give examples of such files then. And add more tests. > > > > > > > > > > > > > > > > You really should try to be more helpful if you care about this > > > code > > > > > > > > working. > > > > > > > > > > > > > > > > > > > > > Code works perfectly from start. There are always attempts to > > > break it. > > > > > > > Your attempts to belittle my work are futile. > > > > > > > > > > > > Perfect code should live in an external repository that is locked > > > > > > against modification. > > > > > > > > > > > > The ffmpeg repository is only for imperfect code that evolves with > > > time, > > > > > > and so requires changes. > > > > > > > > > > > > > > > > > I dunno what Michael attempts to fix. Decoder works fine with valid > > > files. > > > > > I doubt that encoder would encode random bytes or padding into valid > > > file > > > > > bitstream. > > > > > > > > the stride*4 / width*4 change was because of 2 things. > > > > first with AV_PIX_FMT_BGR24 the data stored is not width*4 > > > > > > > > stride is in units of 4 bytes for some reason, so stride*4 > > > > fixes this > > > > The 2nd issue is that the code addresses it by "s->stride * 4" > > > > so the buffer allocation should be stride*4 if we belive the > > > > other code is correct > > > > > > > > src = s->buffer; > > > > ... > > > > for (y = 0; y < avctx->height; y++) { > > > > ... > > > > src += s->stride * 4; > > > > > > > > width*4 works because its bigger than stride*4 for BGR24 which is what > > > all > > > > samples i have use. > > > > > > > > also > > > > ssrc = s->buffer; > > > > ... > > > > for (y = 0; y < avctx->height; y++) { > > > > ... > > > > ssrc += s->stride * 4; > > > > and > > > > dst = (uint32_t *)s->buffer; > > > > > > > > for (block = 0, y = 0; y < s->yb; y++) { > > > > int block_h = s->blocks[block].h; > > > > uint32_t *rect = dst; > > > > > > > > for (x = 0; x < s->xb; x++) { > > > > int block_w = s->blocks[block].w; > > > > uint32_t *row = dst; > > > > > > > > block_h = s->blocks[block].h; > > > > if (s->blocks[block].xor) { > > > > for (k = 0; k < block_h; k++) { > > > > uint32_t *column = dst; > > > > for (l = 0; l < block_w; l++) > > > > *dst++ ^= *src++; > > > > dst = &column[s->stride]; > > > > } > > > > } > > > > dst = &row[block_w]; > > > > ++block; > > > > } > > > > dst = &rect[block_h * s->stride]; > > > > } > > > > > > > > Again, if you have fmvc files with more odd widths or other pixel formats > > > > these would be very welcome. I can just say the code as is in git is > > > wrong > > > > and the buffer size as is in git is wrong. I noticed this when i added > > > > a check to see if the buffer is only partly filled and realized its > > > > always partly filled even when the whole image is actually touched > > > > > > If there are no objections aka noone sees a bug in this then id like > > > to apply this > > > > > > > Since when are partially filled buffers are bad thing? > > - waste of memory > - breaks subsequent patch > - width and stride relate this way: > s->stride = (avctx->width * avctx->bits_per_coded_sample + 31) / 32; > is width always bigger or equal ? > If not we might be accessing outside the array because access uses > stride, allocation width > > Or one line awnser > Since when is it a good thing to mismatch allocation and access? any objections to this ? i think this should be applied thx [...]
diff --git a/libavcodec/fmvc.c b/libavcodec/fmvc.c index 912ad8fc82..de2bf828f4 100644 --- a/libavcodec/fmvc.c +++ b/libavcodec/fmvc.c @@ -614,7 +614,7 @@ static av_cold int decode_init(AVCodecContext *avctx) } s->bpp = avctx->bits_per_coded_sample >> 3; - s->buffer_size = avctx->width * avctx->height * 4; + s->buffer_size = s->stride * avctx->height * 4; s->pbuffer_size = avctx->width * avctx->height * 4; s->buffer = av_mallocz(s->buffer_size); s->pbuffer = av_mallocz(s->pbuffer_size);
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/fmvc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)