heif: add CreateCopy support (WIP) #8907

bradh · 2023-12-04T09:32:14Z

What does this PR do?

Implements CreateCopy for the heif driver. The implementation uses libheif.

This implementation also includes some cleanups to existing code. That could be split out into another PR if desired.

What are related issues/pull requests?

N/A.

Tasklist

Environment

N/A.

rouault · 2023-12-04T14:56:26Z

frmts/heif/heifdrivercore.cpp

@@ -42,33 +42,33 @@ int HEIFDriverIdentifySimplified(GDALOpenInfo *poOpenInfo)
    if (poOpenInfo->nHeaderBytes < 12 || poOpenInfo->fpL == nullptr)
        return false;

+#if LIBHEIF_HAVE_VERSION(1, 4, 0)
+    const auto res =


we can't call libheif file in this file, since for deferred loaded plugins (https://gdal.org/development/rfc/rfc96_deferred_plugin_loading.html), it will be linked into libgdal core, without libheif

Ah. Had not understood, and will need to review the RFC in detail to fix.

Based on an initial review of the RFC, I understand this is a filter that should return true for all supported files. That is, it is probably better to return a false positive (which will delay startup) than a false negative (which will cause the file not to be loaded), within reason. Clearly it needs to be a light test though.

Based on an initial review of the RFC, I understand this is a filter that should return true for all supported files. That is, it is probably better to return a false positive (which will delay startup) than a false negative (which will cause the file not to be loaded), within reason.

yes.
If there are not-sure situations, and mostly if the file might be opened by another driver, you can also return GDAL_IDENTIFY_UNKNOWN, and the Open() method with be invoked.

Implemented. Additional RFC review to do.

frmts/heif/heifdataset.cpp

frmts/heif/heifdatasetcreatecopy.cpp

coveralls · 2023-12-05T06:06:46Z

coverage: 67.998% (+0.001%) from 67.997%
when pulling 3ac750b on bradh:libheif_createcopy_2023-12-03
into fba7ec4 on OSGeo:master.

github-actions · 2024-05-16T02:36:15Z

The GDAL project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 28 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

that all unit tests are passing
that all comments by reviewers have been addressed
that there is enough information for reviewers, in particular link
to any issues which this pull request fixes
that you have written unit tests where possible
In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
If there is no further activity on this pull request, it will be closed in 2 weeks.

bradh · 2024-05-16T04:50:30Z

If there are feedback items on what is currently implemented, that would be appreciated.

rouault

a few comments, but looks good. Would need to be rebased on top of latest master to get fresh CI. Do you feel like this PR could be merged as it (with potential future PRs), or is there some extra work required?

autotest/gdrivers/heif.py

frmts/heif/heifdatasetcreatecopy.cpp

bradh · 2024-05-17T00:48:41Z

a few comments, but looks good. Would need to be rebased on top of latest master to get fresh CI.

Thanks.

Do you feel like this PR could be merged as it (with potential future PRs), or is there some extra work required?

I would not want to merge it without the test cases and documentation, or with the driver check hack. The other parts could probably wait for future PRs.

Co-authored-by: Even Rouault <[email protected]>

rouault · 2024-05-18T14:41:11Z

doc/source/drivers/raster/heif.rst

@@ -31,6 +34,26 @@ Driver capabilities

 .. supports_virtualio:: if libheif >= 1.4

+Creation support


to fix "AssertionError: Driver HEIF declares DCAP_CREATECOPY but doc does not!" in https://github.com/OSGeo/gdal/actions/runs/9139007180/job/25130835649?pr=8907:

Suggested change

Creation support

.. supports_createcopy::

Creation support

Thanks. Already fixed in a separate work branch. Still not sure what is going on with the bus error on the Alpine builds....

on the Alpine builds....

perhaps a issue specific with the libheif version they ship

A way to reproduce locally is to use the Docker alpine:edge image

docker run --name gdal_alpine -it -v /path/to/your/GDAL/checkout:/gdal alpine:edge

execute the apk add line at https://github.com/OSGeo/gdal/blob/master/.github/workflows/alpine/Dockerfile.ci#L3
cd /gdal
python3 -m pip install --break-system-packages -U -r autotest/requirements.txt
mkdir build_alpine
cd build_alpine
do the build as in https://github.com/OSGeo/gdal/blob/master/.github/workflows/alpine/build.sh#L16 (drop the line "-DADD_EXTERNAL_DEFERRED_PLUGIN_FOO=/tmp/foo.cpp" to simplify things)

github-actions · 2024-06-16T02:40:10Z

The GDAL project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 28 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

that all unit tests are passing
that all comments by reviewers have been addressed
that there is enough information for reviewers, in particular link
to any issues which this pull request fixes
that you have written unit tests where possible
In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
If there is no further activity on this pull request, it will be closed in 2 weeks.

github-actions · 2024-06-30T02:40:47Z

While we hate to see this happen, this PR has been automatically closed because it has not had any activity in the last 6 weeks. If this pull request should be reconsidered, please follow the guidelines in the previous comment and reopen this pull request. Or, if you have any further questions, just ask! We love to help, and if there's anything the GDAL project can do to help push this PR forward please let us know how we can assist.

bradh · 2024-06-30T10:49:51Z

I'll rework this and submit as a new PR.

bradh marked this pull request as draft December 4, 2023 09:45

bradh force-pushed the libheif_createcopy_2023-12-03 branch from 03e6545 to ad07518 Compare December 4, 2023 10:22

rouault reviewed Dec 4, 2023

View reviewed changes

bradh force-pushed the libheif_createcopy_2023-12-03 branch from 3ac750b to a65fd32 Compare December 5, 2023 06:39

github-actions bot added the stale label May 16, 2024

stale bot removed the stale label May 16, 2024

rouault reviewed May 16, 2024

View reviewed changes

autotest/gdrivers/heif.py Outdated Show resolved Hide resolved

frmts/heif/heifdatasetcreatecopy.cpp Outdated Show resolved Hide resolved

bradh and others added 14 commits May 18, 2024 17:02

heif: implement CreateCopy (WIP)

b2bc640

heif: only use compression formats that are available in libheif release

827667e

heif: fix errors in deferred driver loading

2637b09

heif: PR review comment fixes, additional tests

410f0bc

heif: back out uncompressed codec

7ab41d6

heif: additional test work

d0d2bd0

heif: implement VFS writer

4208c1a

heif: mark libheif context as unused

10fe629

heif: avoid use of new heif_error_success constant

f19beee

heif: add protection against double inclusion of dataset header file

c610c03

heif: initial creation documentation

0fc6383

Update autotest/gdrivers/heif.py

6d2d8b2

Co-authored-by: Even Rouault <[email protected]>

Update frmts/heif/heifdatasetcreatecopy.cpp

c1bf35a

Co-authored-by: Even Rouault <[email protected]>

heif: correct autotest temporary path

2545967

bradh force-pushed the libheif_createcopy_2023-12-03 branch from fa17bec to 2545967 Compare May 18, 2024 09:23

heif: remove unused import from python tests

d1b3cd5

bradh force-pushed the libheif_createcopy_2023-12-03 branch from 2adfc1f to d1b3cd5 Compare May 18, 2024 09:53

rouault reviewed May 18, 2024

View reviewed changes

github-actions bot added the stale label Jun 16, 2024

github-actions bot closed this Jun 30, 2024

bradh mentioned this pull request Oct 22, 2024

heif: add CreateCopy support #11093

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

heif: add CreateCopy support (WIP) #8907

heif: add CreateCopy support (WIP) #8907

bradh commented Dec 4, 2023 •

edited

Loading

rouault Dec 4, 2023

bradh Dec 4, 2023

rouault Dec 4, 2023

bradh Dec 5, 2023

coveralls commented Dec 5, 2023

github-actions bot commented May 16, 2024

bradh commented May 16, 2024

rouault left a comment

bradh commented May 17, 2024

rouault May 18, 2024

bradh May 18, 2024

rouault May 18, 2024

github-actions bot commented Jun 16, 2024

github-actions bot commented Jun 30, 2024

bradh commented Jun 30, 2024

		@@ -31,6 +34,26 @@ Driver capabilities

		.. supports_virtualio:: if libheif >= 1.4

		Creation support

heif: add CreateCopy support (WIP) #8907

heif: add CreateCopy support (WIP) #8907

Conversation

bradh commented Dec 4, 2023 • edited Loading

What does this PR do?

What are related issues/pull requests?

Tasklist

Environment

rouault Dec 4, 2023

Choose a reason for hiding this comment

bradh Dec 4, 2023

Choose a reason for hiding this comment

rouault Dec 4, 2023

Choose a reason for hiding this comment

bradh Dec 5, 2023

Choose a reason for hiding this comment

coveralls commented Dec 5, 2023

github-actions bot commented May 16, 2024

bradh commented May 16, 2024

rouault left a comment

Choose a reason for hiding this comment

bradh commented May 17, 2024

rouault May 18, 2024

Choose a reason for hiding this comment

bradh May 18, 2024

Choose a reason for hiding this comment

rouault May 18, 2024

Choose a reason for hiding this comment

github-actions bot commented Jun 16, 2024

github-actions bot commented Jun 30, 2024

bradh commented Jun 30, 2024

bradh commented Dec 4, 2023 •

edited

Loading