Missing source for shaders

sebastinas commented 7 years ago

It appears as if the released tarball and the git repository do not contain sources for all (compiled) shaders. For the 1.8.3 tarballs the sources are missing for

src/shaders/brc/bsw/hme_downscale_genx_0.g8b
src/shaders/brc/bsw/hme_genx_0.g8b
src/shaders/brc/bsw/vp8_brc_init_genx_0.g8b
src/shaders/brc/bsw/vp8_brc_reset_genx_0.g8b
src/shaders/brc/bsw/vp8_brc_update_genx_0.g8b
src/shaders/brc/bsw/vp8_enc_genx_0.g8b
src/shaders/brc/bsw/vp8_enc_genx_1.g8b
src/shaders/brc/bsw/vp8_enc_genx_2.g8b
src/shaders/brc/bsw/vp8_intra_distortion_genx_0.g8b
src/shaders/brc/bsw/vp8_mpu_genx_0.g8b
src/shaders/brc/bsw/vp8_tpu_genx_0.g8b
src/shaders/brc/skl/hme_downscale_genx_0.g9b
src/shaders/brc/skl/hme_genx_0.g9b
src/shaders/brc/skl/vp8_brc_init_genx_0.g9b
src/shaders/brc/skl/vp8_brc_reset_genx_0.g9b
src/shaders/brc/skl/vp8_brc_update_genx_0.g9b
src/shaders/brc/skl/vp8_enc_genx_0.g9b
src/shaders/brc/skl/vp8_enc_genx_1.g9b
src/shaders/brc/skl/vp8_enc_genx_2.g9b
src/shaders/brc/skl/vp8_intra_distortion_genx_0.g9b
src/shaders/brc/skl/vp8_mpu_genx_0.g9b
src/shaders/brc/skl/vp8_tpu_genx_0.g9b
src/shaders/h264/mc/avc_mc.g4b
src/shaders/post_processing/gen75/sharpening_h_blur.g75b
src/shaders/post_processing/gen75/sharpening_unmask.g75b
src/shaders/post_processing/gen75/sharpening_v_blur.g75b
src/shaders/post_processing/gen8/conv_nv12.g8b
src/shaders/post_processing/gen8/sharpening_h_blur.g8b
src/shaders/post_processing/gen8/sharpening_unmask.g8b
src/shaders/post_processing/gen8/sharpening_v_blur.g8b
src/shaders/post_processing/gen9/conv_nv12.g9b
src/shaders/post_processing/gen9/conv_p010.g9b
src/shaders/post_processing/gen9/pa_to_pa.g9b
src/shaders/post_processing/gen9/pa_to_pl2.g9b
src/shaders/post_processing/gen9/pa_to_pl3.g9b
src/shaders/post_processing/gen9/pl2_to_pa.g9b
src/shaders/post_processing/gen9/pl2_to_pl2.g9b
src/shaders/post_processing/gen9/pl2_to_pl3.g9b
src/shaders/post_processing/gen9/pl2_to_rgbx.g9b
src/shaders/post_processing/gen9/pl3_to_pa.g9b
src/shaders/post_processing/gen9/pl3_to_pl2.g9b
src/shaders/post_processing/gen9/pl3_to_pl3.g9b
src/shaders/post_processing/gen9/rgbx_to_nv12.g9b

(List taken from https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=867340)

Also, when looking at the current master branch, some sources are missing. If you one deletes all compiled shaders contained in the repository, at least some are not re-generated:

src/shaders/brc/bsw/vp8_brc_init_genx_0.g8b
src/shaders/brc/skl/vp8_brc_init_genx_0.g9b
src/shaders/post_processing/gen75/sharpening_h_blur.g75b
src/shaders/post_processing/gen9/conv_p010.g9b
src/shaders/h264/mc/avc_mc.g4b
src/shaders/post_processing/gen5_6/null.g6b
src/shaders/post_processing/gen8/conv_nv12.g8b

(Listen taken from find -name "*.g*b" -delete; autoreconf -vfi; ./configure; make -i).

Please ensure that all sources are contained in the git repository and the released tarballs.

QuPengfei commented 7 years ago

@sebastinas That is true there are no source codes for some shaders. it have been long time. Actually, for some reason, some source of shader are private and not allowed to upstream. Maybe you can understand this situation.

sebastinas commented 7 years ago

This is unfortunate and a very serious issue for Debian (see DFSG) and we need to resolve this issue somehow. The easiest solution for us - of course - would be the inclusion of the respective sources. But if this is impossible, as you say, Debian has some possible ways forward:

We strip shaders without source and disable the code paths that depend on those shaders. So the question is, which features we would loose by simply removing them. But assuming that this would only affect the encoders, we could probably live with that and direct any users that need hardware accelerated encoding to you.
We strip shaders without source, but move it to a non-free package similar to what we do for the kernel and proprietary firmware.
We remove the driver from Debian.

The last option is the least desirable one as it's a disservice to our users. 1. is probably the easiest one to implement for us provided that the shaders are not essential for decoding. 2. would be doable but would take some time.

So let's start with the easy questions: if we'd remove the affected shaders, what would break?

In any case, it'd be great if we could fix this issue somehow without too much inconvenience to our users. I'll also talk to the co-maintainers of the package in Debian. Maybe they have some other ideas on possible ways forward.

sebastinas commented 7 years ago

Looks like we also need to add src/gen9_avc_encoder_kernels.c, src/gen9_hevc_enc_kernels_binary.c and src/gen9_vpc9_encoder_kernels.c to the list.

QuPengfei commented 7 years ago

@sebastinas those shaders mainly affect the encoder,such as VP8/AVC/HEVC.

For solution 2,

is there guide how to move the shaders without source into a non-free package? what is the package name and where to put？
what does this solution affect the users or customers?

sebastinas commented 7 years ago

I don't think there is a guide I can point you to. But what we basically need would be something like:

The shader package installs all the shaders to same fixed path, say $datadir/i965-va-driver-shaders (if the shaders are architecture independent) or $libdir/i965-va-driver-shaders (if the the shaders are architecture dependent). The choice probably also depends on the format of the data.
- If you just want to move the symbols defining the shaders to a different so, I'd put them somewhere in $libdir/i964-va-driver-shaders, dlopen the file during initialization of the driver and get the shaders with dlsym if available. Given the current representation of the shaders, this looks like a relatively easy solution to implement.
- Another option would be to store the shaders in some file and then fopen and fread them during startup.
In case loading of any of the shaders fail, the driver could then simply not offer VP8/AVC/HEVC encoding.

Users then need to install one extra package to get the encoders. The existing package can suggest the new package and we will document in Debian's release notes, that users how need support for hardware accelerated encoding need to install this extra package.

sebastinas commented 6 years ago

Is there any progress on this issue? Do you need help with implementing a solution?

We should get this fixed before the release of 2.0.0. This could become a major blocker otherwise.

xhaihao commented 6 years ago

You will be appreciated if you could help to implement it. BTW this is not an API/ABI compatibility issue, so I'd like to fix it in 2.x.x, not in 2.0.0.

sebastinas commented 6 years ago

You can follow the progress at https://github.com/sebastinas/intel-vaapi-driver/tree/external-kernels

sebastinas commented 6 years ago

FWIW, a build with that patch set is not available in the Debian archive. The extra shared library is still awaiting review by our FTP masters.

sebastinas commented 6 years ago

@xhaihao So what's the status of this issue? I see new shaders merged without source instead of this issue getting fixed.

montvid commented 6 years ago

Any news?

sebastinas commented 5 years ago

This is my current branch: https://github.com/sebastinas/intel-vaapi-driver/tree/2.2.0-remove-non-free-shaders. I'd love some feedback - especially with respect to the removed .has_XYZ flags.

intel / intel-vaapi-driver

Missing source for shaders #220