[MIR] libheif

I've had a look, certainly seems fine packaging-wise; but should be looked at by the Security Team.

I just noticed via https://people.canonical.com/~ubuntu-archive/component-mismatches-proposed.svg that libde265 wants to pull in:
 - qtbase-opensource-src (qt4)
  - qtchooser
  - double-conversion
  - qtsvg-opensource-src
  - qttranslations-opensource-src
 - libsd1.2
 - *ffmpeg*
  - zeromq3
   - norm
   - libpgm
  - libdc1394-22
  - libbs2b
  - x264
  - lilv
   - sratom
   - serd
   - sord
  - zvbi
  - libva
   - intel-vaapi-driver
   - intel-media-driver
    - intel-gmmlib
   - libset-scalar-perl
  - codec2
  - chromaprint
  - twitter-bootstrap3
  - libgsm
  - crystalhd
  - libopenmpt
  - game-music-emu
  - libvidstab
  - rubberband
  - aom
  - xvidcore
  - openjpeg2
  - libbluray
   - libaacs
    - libbdplus
  - openal-soft
   - sndio
  - shine
  - flite
  - libmysofa
  - libass

Sooo that's a huge pile of work and it's not justifiable in terms of reducing delta on the imagemagick package. I'm going to reupload imagemagick to drop the libheif build-dependency. Security feedback on the suitability of libheif is still welcome but is a low priority.

Wow, that's a pretty significant chain of deps. Thanks for pruning this one.

I had started in on libheif but not much beyond inspecting a few Coverity results. (More in the spirit of learning Coverity than inspecting libheif, I must be honest.)

I passed along the Coverity results to upstream on https://github.com/strukturag/libheif/issues/128 -- hopefully it'll be useful to them.

Thanks

libde265-0 actually pulls in a lot less dependencies:
https://packages.ubuntu.com/bionic/libde265-0

The dependencies on Qt, SDL, swscale are being pulled in by the examples:
https://packages.ubuntu.com/bionic/libde265-examples

libheif only depends on libde265-0 and thus doesn't pull in the whole list of dependencies mentioned in #2

Joachim, thanks for bringing this to my attention. I've gone ahead and added libde265-examples to the exclusion list for main, and readded the libheif build-dep to imagemagick in order to revisit this MIR.

Seth, can this go back in the queue for security team review?

Thanks Joachim, Steve, I've removed the comment in trello saying this is on hold.

Thanks

Just a question, I thought build-dependencies need not be in main any more to build packages in main. It was announced some time ago on ubuntu-devel. Am I wrong?

It's correct that packages do not have to be in main merely for being build dependencies. But most libraries that are build dependencies are linked against during build and become runtime dependencies.

This MIR keeps imagemagick out of the release pocket in the devel release.

Apparently this is useful for photos from iPhones.

Also the Samsung S10 creates HEIF images:
https://r2.community.samsung.com/t5/Galaxy-S/Here-s-how-to-make-Galaxy-S10-camera-photos-take-less-storage/td-p/1304386

Status changed to 'Confirmed' because the bug affects multiple users.

Here's the Coverity results on libheif. The first third of the file is all about example code. It might be nice to get those issues fixed, both to help bring the overall number of findings closer to zero, but more importantly to improve the quality of potential copy-and-paste coding.

I haven't looked at the results in any real depth.

Thanks

I forgot I already passed along the coverity results to the libheif team:
https://github.com/strukturag/libheif/issues/128

Their response is very encouraging.

Thanks

Is the libheif package still lintian clean? Using disco's lintian I get the following:

Output of lintian:
 W: libheif source: debhelper-compat-file-is-missing
 W: libheif source: package-uses-deprecated-debhelper-compat-version 1
 E: libheif source: package-uses-debhelper-but-lacks-build-depends
 W: libheif source: newer-standards-version 4.4.1 (current is 4.1.4)

This is libheif 1.6.1-1.

Thanks

Has anyone looked to see if x265 and libde265 are also required to be MIR'd for this package to be useful in its target use? There's no install time dependencies declared in the Debian packaging but it's hard for me to imagine why these libraries would be required at build in order to enable some of the functionality, unless they also influence the runtime behaviour of the library.

Thanks

Seth, I'm not sure how you reached the conclusion that there are "no install time dependencies declared in the Debian packaging". libheif1-1 in focal depends on both libx265-179 and libde265-0.

Thanks Steve, I'm not sure why I thought the dependencies on libx265-179 and libde265-0 were overlooked. Now that I take another look it all looks pretty normal.

Thanks

> Is the libheif package still lintian clean?

Looks like that lintian is too old.

I've returned to this package, and I'm seeing enough // TODO markers that I'm starting to be concerned that this library implements sufficient functionality to be useful for us. The code that's there mostly looks good, but there's enough markers for "handle errors" and "find out .." that I'm wondering if enough of this is useful for the things we want it for.

Has it been used and found useful?

Thanks

On Wed, Feb 26, 2020 at 04:02:33AM -0000, Seth Arnold wrote:
> Has it been used and found useful?

Not by me. This was pulled into Ubuntu automatically via Debian.

I reviewed libheif 1.6.1-1 as checked into focal. This shouldn't be
considered a full audit but rather a quick gauge of maintainability.

libheif is an image codec library necessary for decoding photos from some
newer phones.

- CVE History:
  - CVE-2019-11471, our database says still unfixed in 18.04 LTS.
- Build-Depends: debhelper-compat, libde265-dev, libgdk-pixbuf2.0-dev,
  libjpeg-dev, libpng-dev, libx265-dev, pkg-config
- no pre/post inst/rm scripts
- no init scripts
- no systemd units
- no dbus services
- no setuid binaries
- binaries in PATH
  - heif-thumbnailer in heif-thumbnailer
  - heif-convert, heif-enc, heif-info in libheif-examples
- no sudo fragments
- no udev rules
- There are some very thin unit tests. No autopkgtests. THere's some
  support for fuzzers but I don't see it used.
- no cron jobs
- relatively clean build logs

- no processes spawned
- significant memory management and C-style manipulation of data. Most
  calls looked like there were checks in place, but this level of C-style
  memory manipulation probably has errors.
- No file IO in library, only in examples
- Very little human logging; looked fine
- No environment variable usage
- No use of privileged functions
- No cryptography
- No temp files
- No networking
- No webkit
- No polkit

- cppcheck false positive
- an earlier look found some coverity issues, which the team addressed

Here's a list of the small handful of things I noticed:

In convert_libde265_image_to_heif_image() what constrains stride to
reasonable values? I get lost reading libde265 code to find the stride.

setjmp() used for error handling in example code; this kind of error
handling is very difficult to use correctly over time.

Y4MEncoder::Encode() doesn't appear to guard against integer overflow in
fwrite() calls

Box_iloc::write_mdat_after_iloc() 4gig outputs unhandled

I'm not sure the consequences of any of these issues.

Code quality looked goodh, especially for a codec library; the examples didn't
look as good, but this is common.

Security team ACK for promoting the libheif library packages libheif1 and
heif-gdk-pixbuf to main. I'd like to keep the examples in heif-thumbnailer
and libheif-examples in universe.

Thanks

Anything we could do to help move this forward?

It's unclear to me if this ever was approved by the MIR team, possibly comment 1 was intended to be a MIR team review, but it's not clear if that was the intention and certainly doesn't have the normal MIR review details.

Technically after security team review, this should have been changed to status 'In Progress' to wait for seeding updates, but I think this got 'lost' in the process since it was in state 'CONFIRMED' but the MIR team has only been looking at 'NEW' state unclaimed bugs.

@paelzer I updated the 'All open unclaimed MIR bugs' link on the MIR wiki page to show both NEW and CONFIRMED bugs, and I think we should be sure to include 'CONFIRMED' state bugs in our weekly review, not just 'NEW' state bugs.

> @paelzer I updated the 'All open unclaimed MIR bugs' link on the MIR
> wiki page to show both NEW and CONFIRMED bugs, and I think we should be
> sure to include 'CONFIRMED' state bugs in our weekly review, not just
> 'NEW' state bugs.

Yeah I've got a notification about it.
I agree and thank you Dan!
I have shortened the link (same function, but omitting the defaults)
and updated also the link that we use in the weekly meeting (also on
that page, but in a different place).
Finally I have updated the state-machine info on the page to reflect
that we now also consider "Confirmed" tasks in our incoming queue.

We'll have to re-review all these cases that pop up by that, they are
all a bit fallen through the cracks by this.
Once discussed we will update each of one either assigning a
(re-)reviewer or whatever else seems to be the appropriate next step.

Per comment #1 + #24 libheif is ready - updating the state.
But the dependencies are not yet looked at in depth at all.

We will need to look for it, but first we need to make sure (likely but not state explicitly yet) that someone wants to look after it.
I assume that foundations will own them, but would like to ask for a MIR-request template for x265 and libde265 in the description please.
That will also inlcude the QA-checks that the requester is supposed to do as part of [1].
Setting them to incomplete to reflect that.
Please set them back to NEW once this is ready.

Then (probably next week meeting if it is ready by the time) we assigned them for review (and if generally ok they most likely will have to go to security review as well).

[1]: https://wiki.ubuntu.com/MainInclusionProcess?action=show&redirect=MIRTeam#Filing_a_MIR_bug

imagemagick is no longer in main, so this is wontfix.

libheif is now a dependency of libgd2 in lunar-proposed, so asking for reconsideration of this MIR. (For the moment I am dropping the added dependency from libgd2 to unblock it.)

Setting the libheif status to "New", so it pops up in the correct MIR queue.

Overall, this MIR is pretty old and we'll probably want to have it updated using the new template in order to do a new review.

https://github.com/canonical/ubuntu-mir#main-inclusion-requirements

Will the mentioned "updated using the new template" come?
Marking it incomplete until done

> Will the mentioned "updated using the new template" come?

I guess this comment is addressed to me as the submitter. It wasn't clear to me that the previous comment was.

The MIR was already approved for libheif (though not its dependencies) before being abandoned with the determination that it was no longer needed. I don't think it's reasonable to require resubmission with fresh paperwork given that it was already approved. We aren't requiring re-review of all the other packages already in main.

Hi,
sorry for being misunderstandable.
I agree, that we do not need anything more for libheif itself (which should be in "in progress" to reflect it is ready, but no change has landed that would pull it in).

But as we all have seen its dependencies aom, dav1d, libde265, x265 are not yet processed.
They will need:
a) initial paperwork (other than rationale they usually share nothing, so they are per PKG)
b) a team that commits to own them
c) set those tasks back to "new" to enter the MIR process queue

So they are "incomplete" waiting for one to step up committing to (b) and providing (a) and doing (c) for them to enter the queue.

FYI: I have just released v1.4.0 of libheif.
A main change in that version is that it removes all dependencies (except to the new zlib dependency) and uses a dynamic plugin system.
I.e. all the codecs (aom, dav1d, ...) will be moved to separate packages.

That means that in order to have a working installation with only AVIF support, it is sufficient to install the codec package with the aom dependency.
For HEIC support, we will need the codecs with libde265 and x265.

There will also be codec plugins for dav1d, rav1e, and svt-av1, but these will be only "Suggested".

Sorry, typo: v1.14.0 is the new version

That is great to know Dirk, as it would cut the recommended (and therefore required to promote) dependencies down quite a bit.
Thanks!

Fur the curious, have a look at https://github.com/strukturag/libheif/releases/tag/v1.14.0

And for the chance that one of the libs will pass and another will fail we might now make more things a suggest in Ubuntu to keep those that do not pass the checks in universe until being ok.

1.14.2-1 is now in lunar.

However, libheif 1.14.2-1 still has all the same dependencies.

Package: libheif1
Version: 1.14.2-1
Depends: libaom3 (>= 3.2.0), libc6 (>= 2.34), libdav1d6 (>= 0.1.0), libde265-0 (>= 1.0.7), libgcc-s1 (>= 3.3.1), libstdc++6 (>= 11), libx265-199 (>= 3.5), zlib1g (>= 1:1.1.4)

And libheif1 contains only the single libheif.so.1.14.2 object.

Did the dynamic plugin system not happen?

Looks like the Debian maintainer deliberately shipped these bundled instead of as plugins.

+ -DPLUGIN_DIRECTORY=/usr/lib/$(DEB_HOST_MULTIARCH)/libheif/plugins \
+ -DWITH_AOM_DECODER_PLUGIN=OFF \
+ -DWITH_AOM_ENCODER_PLUGIN=OFF \
+ -DWITH_DAV1D_PLUGIN=OFF \
+ -DWITH_DEFLATE_HEADER_COMPRESSION=ON \
+ -DWITH_LIBDE265_PLUGIN=OFF \
+ -DWITH_X265_PLUGIN=OFF

I'll reach out to Debian to see what they think about splitting these into a separate package.

Since it looks like the only dependency here which is genuinely optional for proper functionality of the library is dav1d, and the other dependencies are essential for decoding as mentioned in comment #35, I think we should just investigate MIR of all the dependencies as-is as a first course of action.

Some plugins were still using internal APIs, causing errors while loading (see https://github.com/strukturag/libheif/issues/745). Thats why I decided to keep the Debian packaging as-is for now.

The issue will be fixed in the next version of libheif and Debian will split the packaging into individual plugin packages.

Differences between getting orig tar through rules vs uscan

No lintian output generated when running `lintian --pedantic` except distribution mismatch warning
```
 libheif1: changelog-distribution-does-not-match-changes-file unstable != kinetic [usr/share/doc/libheif1/changelog.Debian.gz:1]
W: libheif changes: distribution-and-changes-mismatch kinetic unstable

```

See separate MIR bug reports for libheif dependencies:

- aom: bug #2004442
- dav1d: bug #2004446
- libde265: bug #2004449
- x265: bug #2004453

Since x265 was NAKd, is this and related MIRs still valid?

libheif depends aom [0] which is incomplete. MIR and Security have conditionally ACK'd, and owning team needs to satisfy conditions.

aom depends on libyuv [1] which is incomplete. MIR team conditionally ACK'd. When conditions are satisfied by owning team, the MIR will be assigned to Security.

aom depends on libwebm [2] which Ubuntu Security will ACK soon.

libheif depends on dav1d [3] which is incomplete and not yet ready for MIR team review.

libheif depends on libde265 [4] which needs an Ubuntu Security review.

libheif depends on x265 [5] which was NACKd by Ubuntu Security.

[0] https://bugs.launchpad.net/ubuntu/+source/aom/+bug/2004442
[1] https://bugs.launchpad.net/ubuntu/+source/libyuv/+bug/2004516
[2] https://bugs.launchpad.net/ubuntu/+source/libwebm/+bug/2004523
[3] https://bugs.launchpad.net/ubuntu/+source/dav1d/+bug/2004446
[4] https://bugs.launchpad.net/ubuntu/+source/libde265/+bug/2004449
[5] https://bugs.launchpad.net/ubuntu/+source/x265/+bug/2004453

Note that libheif packaging has changed in version 1.16.2-2.
Codecs are now split out into separate plugin packages.

We now have:
- libheif1 - main library
- libheif-examples - command line tools
- libheif-dev
- heif-gdk-pixbuf
- heif-thumbnailer

and the codecs:
- libheif-plugin-aomdec - AVIF decoder (based on libaom)
- libheif-plugin-aomenc - AVIF encoder (based on libaom)
- libheif-plugin-dav1d - alternative AVIF decoder (based on dav1d)
- libheif-plugin-rav1e - alternative AVIF encoder (based on rav1e)
- libheif-plugin-svtenc - alternative AVIF encoder (based on svt-av1)

- libheif-plugin-libde265 - HEIC decoder (based on libde265)
- libheif-plugin-x265 - HEIC encoder (based on x265)

In the next release, there will also be support for "kvazaar" as an alternative HEIC encoder (instead of x265).

In order to be useful to the user:
- for AVIF support, the two codec packages based on libaom work best for me and are well maintained.
- for HEIC decoding support, the libde265 plugin package is required.

As x265 is NACKd, one could leave this out (no encoding support for HEIC) and replace this with 'kvazaar' in the future.

Thank you @dirk-farin!

Glad to hear that kvazaar is a possible alternative. I'll run a quick audit in case we can report something to upstream early.

It looks like rust-rav1e and svt-av1 will need MIRs. For preparing Rust MIRs, please see https://wiki.ubuntu.com/RustCodeInMain

Thank you both Dirk and Mark for coming up with a single coherent view of where this is at today.

Indeed, thanks for the summary. Effectively, for an MVP (libheif with HEIC decoding support), we'd need to satisfy the following depends:

* libheif-plugin-dav1d OR libheif-plugin-aomdec
=> bug #2004442 (src:aom) + bug #2004516 (src:libyuv) + bug #2004523 (src:libwebm)

* libheif-plugin-libde265
=> bug #2004449 (src:libde265)

It sounds like getting aom (and its transitive depends) MIRed should be doable, same for libde265. Everything else can be ignored for now, thanks to the new plugin architecture, and we can revisit in the future to enable more functionality/plugins.

On Thu, Aug 17, 2023 at 07:26:01AM -0000, Lukas Märdian wrote:
> Indeed, thanks for the summary. Effectively, for an MVP (libheif with
> HEIC encoding support), we'd need to satisfy the following depends:

I think you mean decoding support here

> I think you mean decoding support here

ACK, fixed.