Y4M loader: use custom y4m parser #14

Closed
NRK wants to merge 1 commits from NRK:y4m into master
First-time contributor

avoid dependency on liby4m, which has some quality issues and isn't
available on any distros according to repology.

additionally, the new loader now supports:

  • loading from memory
  • multi-frame support
  • mono colourspace y4m images

Fixes: #13

avoid dependency on liby4m, which has some quality issues and isn't available on any distros according to repology. additionally, the new loader now supports: * loading from memory * multi-frame support * mono colourspace y4m images Fixes: https://git.enlightenment.org/old/legacy-imlib2/issues/13
NRK reviewed 2023-06-19 15:26:22 -07:00
NRK left a comment
Author
First-time contributor

Sample multi-frame y4m: https://samples.mplayerhq.hu/yuv4mpeg2/example.y4m.bz2

Sample mono y4m: ffmpeg -i test/images/icon-64.png -pix_fmt gray8 mono.y4m

Sample multi-frame y4m: https://samples.mplayerhq.hu/yuv4mpeg2/example.y4m.bz2 Sample mono y4m: `ffmpeg -i test/images/icon-64.png -pix_fmt gray8 mono.y4m`
@ -128,0 +347,5 @@
pf = __imlib_GetFrame(im);
if (!pf)
return LOAD_OOM;
pf->frame_count = fcount;
pf->loop_count = 0; /* TODO wtf is this thing? */
Author
First-time contributor

Oops, forgot to fix this up. I'll look into what this is tomorrow :)

Oops, forgot to fix this up. I'll look into what this is tomorrow :)
NRK marked this conversation as resolved
NRK force-pushed y4m from 31bba57db8 to 85d3bdc22b 2023-06-20 01:42:39 -07:00 Compare
Owner

Looks good to me.
Is it ready for merging?
Can I squash the two commits?

Looks good to me. Is it ready for merging? Can I squash the two commits?
Author
First-time contributor

Is it ready for merging?

Yeah, everything works fine as far as I tested. Wanted to remove dependency on libyuv as well, but that'll have to wait for some other day :)

Can I squash the two commits?

Okay by me.

> Is it ready for merging? Yeah, everything works fine as far as I tested. Wanted to remove dependency on libyuv as well, but that'll have to wait for some other day :) > Can I squash the two commits? Okay by me.
Owner

I just got this:

make[4]: Entering directory '/local/stuff/src/E/git/legacy/imlib2/build/cast-align/src/modules/loaders'
/bin/sh ../../../libtool --tag=CC --tag=disable-static --mode=compile gcc -DHAVE_CONFIG_H -I. -I../../../../../src/modules/loaders -I../../.. -I../../.. -I../../../../../src/lib -D PACKAGE_DATA_DIR="/usr/share/imlib2" -W -Wall -Wcast-align -Wpointer-arith -Wshadow -Wwrite-strings -Wno-unused-parameter -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Werror -fvisibility=hidden -g -O2 -Wcast-align=strict -fsanitize=undefined -MT y4m_la-loader_y4m.lo -MD -MP -MF .deps/y4m_la-loader_y4m.Tpo -c -o y4m_la-loader_y4m.lo test -f 'loader_y4m.c' || echo '../../../../../src/modules/loaders/'loader_y4m.c
libtool: compile: gcc -DHAVE_CONFIG_H -I. -I../../../../../src/modules/loaders -I../../.. -I../../.. -I../../../../../src/lib -D PACKAGE_DATA_DIR="/usr/share/imlib2" -W -Wall -Wcast-align -Wpointer-arith -Wshadow -Wwrite-strings -Wno-unused-parameter -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Werror -fvisibility=hidden -g -O2 -Wcast-align=strict -fsanitize=undefined -MT y4m_la-loader_y4m.lo -MD -MP -MF .deps/y4m_la-loader_y4m.Tpo -c ../../../../../src/modules/loaders/loader_y4m.c -fPIC -DPIC -o .libs/y4m_la-loader_y4m.o
../../../../../src/modules/loaders/loader_y4m.c: In function 'y4m_parse_frame':
../../../../../src/modules/loaders/loader_y4m.c:296:48: error: 'sdiv' may be used uninitialized [-Werror=maybe-uninitialized]
296 | res->u_stride = res->v_stride = res->w / sdiv;
| ^
../../../../../src/modules/loaders/loader_y4m.c:233:33: note: 'sdiv' was declared here
233 | ptrdiff_t npixels, sdiv, voff;
| ^~~~
../../../../../src/modules/loaders/loader_y4m.c:295:20: error: 'voff' may be used uninitialized [-Werror=maybe-uninitialized]
295 | res->v = p + voff;
| ^~~~
../../../../../src/modules/loaders/loader_y4m.c:233:39: note: 'voff' was declared here
233 | ptrdiff_t npixels, sdiv, voff;
| ^~~~
cc1: all warnings being treated as errors

Looks like bogus warnings and seem to be triggered by some combination of options.
I'd like not to get them though :)
I can fix or you do. Got to go now though.

I just got this: make[4]: Entering directory '/local/stuff/src/E/git/legacy/imlib2/build/cast-align/src/modules/loaders' /bin/sh ../../../libtool --tag=CC --tag=disable-static --mode=compile gcc -DHAVE_CONFIG_H -I. -I../../../../../src/modules/loaders -I../../.. -I../../.. -I../../../../../src/lib -D PACKAGE_DATA_DIR=\"/usr/share/imlib2\" -W -Wall -Wcast-align -Wpointer-arith -Wshadow -Wwrite-strings -Wno-unused-parameter -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Werror -fvisibility=hidden -g -O2 -Wcast-align=strict -fsanitize=undefined -MT y4m_la-loader_y4m.lo -MD -MP -MF .deps/y4m_la-loader_y4m.Tpo -c -o y4m_la-loader_y4m.lo `test -f 'loader_y4m.c' || echo '../../../../../src/modules/loaders/'`loader_y4m.c libtool: compile: gcc -DHAVE_CONFIG_H -I. -I../../../../../src/modules/loaders -I../../.. -I../../.. -I../../../../../src/lib -D PACKAGE_DATA_DIR=\"/usr/share/imlib2\" -W -Wall -Wcast-align -Wpointer-arith -Wshadow -Wwrite-strings -Wno-unused-parameter -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Werror -fvisibility=hidden -g -O2 -Wcast-align=strict -fsanitize=undefined -MT y4m_la-loader_y4m.lo -MD -MP -MF .deps/y4m_la-loader_y4m.Tpo -c ../../../../../src/modules/loaders/loader_y4m.c -fPIC -DPIC -o .libs/y4m_la-loader_y4m.o ../../../../../src/modules/loaders/loader_y4m.c: In function 'y4m_parse_frame': ../../../../../src/modules/loaders/loader_y4m.c:296:48: error: 'sdiv' may be used uninitialized [-Werror=maybe-uninitialized] 296 | res->u_stride = res->v_stride = res->w / sdiv; | ^ ../../../../../src/modules/loaders/loader_y4m.c:233:33: note: 'sdiv' was declared here 233 | ptrdiff_t npixels, sdiv, voff; | ^~~~ ../../../../../src/modules/loaders/loader_y4m.c:295:20: error: 'voff' may be used uninitialized [-Werror=maybe-uninitialized] 295 | res->v = p + voff; | ~~^~~~~~ ../../../../../src/modules/loaders/loader_y4m.c:233:39: note: 'voff' was declared here 233 | ptrdiff_t npixels, sdiv, voff; | ^~~~ cc1: all warnings being treated as errors Looks like bogus warnings and seem to be triggered by some combination of options. I'd like not to get them though :) I can fix or you do. Got to go now though.
Author
First-time contributor

What version of GCC is that? I don't get that warning (gcc 12.1.1) and that's clearly false positive since the only case those variable aren't initialized is if the color space was mono. (But if the color space was mono, those variables aren't used at all).

I guess you can initialize them to 0 at the top, it should avoid the warnings.

What version of GCC is that? I don't get that warning (gcc 12.1.1) and that's clearly false positive since the only case those variable aren't initialized is if the color space was mono. (But if the color space was mono, those variables aren't used at all). I guess you can initialize them to 0 at the top, it should avoid the warnings.
NRK force-pushed y4m from 2a314057c4 to 2cea59b3da 2023-06-21 08:40:32 -07:00 Compare
Author
First-time contributor

Squashed the commits and hopefully the compiler warning should be fixed now too.

Squashed the commits and hopefully the compiler warning should be fixed now too.
Owner

Warnings gone, pushed, thanks :)

I'm on gcc 13.1.1. Looks like -fsanitize=undefined triggers it.

Warnings gone, pushed, thanks :) I'm on gcc 13.1.1. Looks like -fsanitize=undefined triggers it.
NRK closed this pull request 2023-06-21 10:56:14 -07:00
NRK deleted branch y4m 2023-06-22 07:50:05 -07:00
Author
First-time contributor

@kw Also just noticed, the tests should now allow y4m memory as well. Attached a patch.

@kw Also just noticed, the tests should now allow y4m memory as well. Attached a patch.
Owner

Right, forgot, pushed, thanks :)

Right, forgot, pushed, thanks :)

Pull request closed

Sign in to join this conversation.
No reviewers
No Label
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: old/legacy-imlib2#14
No description provided.