Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix/add exceptions for BMFF based files #2364

Merged
merged 3 commits into from
Oct 3, 2022
Merged

Fix/add exceptions for BMFF based files #2364

merged 3 commits into from
Oct 3, 2022

Conversation

postscript-dev
Copy link
Collaborator

No description provided.

@postscript-dev postscript-dev added bug imageHandler Anything related to specific ImageHandlers architecture labels Sep 24, 2022
@postscript-dev postscript-dev self-assigned this Sep 24, 2022
@codecov
Copy link

codecov bot commented Sep 24, 2022 •
edited
Loading

Codecov Report

Merging #2364 (f089398) into main (640b0fb) will increase coverage by 0.62%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #2364      +/-   ##
==========================================
+ Coverage   63.51%   64.14%   +0.62%     
==========================================
  Files         119      119              
  Lines       20634    21071     +437     
  Branches    10245    10394     +149     
==========================================
+ Hits        13106    13515     +409     
- Misses       5399     5400       +1     
- Partials     2129     2156      +27
Impacted Files Coverage Δ
include/exiv2/bmffimage.hpp 100.00% <ø> (ø)
src/bmffimage.cpp 75.75% <0.00%> (-1.07%) ⬇️
src/tiffimage_int.cpp 79.58% <0.00%> (-0.25%) ⬇️
src/types.cpp 94.44% <0.00%> (-0.04%) ⬇️
src/easyaccess.cpp 93.10% <0.00%> (ø)
src/makernote_int.cpp 65.39% <0.00%> (+0.76%) ⬆️
src/quicktimevideo.cpp 58.15% <0.00%> (+1.18%) ⬆️
src/tags_int.hpp 91.17% <0.00%> (+2.80%) ⬆️
src/minoltamn_int.cpp 60.98% <0.00%> (+5.92%) ⬆️
src/sonymn_int.cpp 77.36% <0.00%> (+19.15%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

piponazo
piponazo previously approved these changes Sep 27, 2022
View reviewed changes
Copy link
Collaborator

@piponazo piponazo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine for me to change the exception type 👍

@mergify mergify bot dismissed piponazo’s stale review October 1, 2022 14:26

Pull request has been modified.

@postscript-dev postscript-dev changed the title Fix BmffImage::writeMetadata() error id/message Fix/add exceptions for BMFF based files Oct 1, 2022
@postscript-dev
Copy link
Collaborator Author

@piponazo
I have also added exceptions when setting ExifData, IptcData and XmpData as we don't support this in BMFF files. Do I need to add tests to demonstrate this or are you satisfied that the PR is simple enough to approve?

@postscript-dev postscript-dev marked this pull request as ready for review October 1, 2022 15:14
kmilos
kmilos approved these changes Oct 3, 2022
View reviewed changes
Copy link
Collaborator

@kmilos kmilos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kmilos
Copy link
Collaborator

kmilos commented Oct 3, 2022

Please backport to 0.27-maintenance as well.

@postscript-dev postscript-dev merged commit a2cb06a into Exiv2:main Oct 3, 2022
@1div0
Copy link
Collaborator

1div0 commented Oct 3, 2022

Gr8! Thank You so much.

@postscript-dev postscript-dev mentioned this pull request Oct 8, 2022
Merged
postscript-dev added a commit that referenced this pull request Oct 18, 2022
@postscript-dev
Fix bmff exception; Bump Conan version (#2375)
4f1d091 
- This is a partial backport of #2364. The new exceptions in `::setExifData()`/`::setIptcData()`/`::setXmpData()` are not included as they could change the control flow of the maintenance branch for the user.
- Conan change is a backport of #2361
- Closes #2350
@postscript-dev postscript-dev deleted the fix_bmff_throw_msg branch October 18, 2022 11:09
antermin pushed a commit to antermin/exiv2 that referenced this pull request Mar 16, 2023
@postscript-dev @antermin
Fix bmff exception; Bump Conan version (Exiv2#2375)
2de65f3 
- This is a partial backport of Exiv2#2364. The new exceptions in `::setExifData()`/`::setIptcData()`/`::setXmpData()` are not included as they could change the control flow of the maintenance branch for the user.
- Conan change is a backport of Exiv2#2361
- Closes Exiv2#2350
@nehaljwani nehaljwani mentioned this pull request May 8, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kmilos kmilos kmilos approved these changes

@piponazo piponazo piponazo left review comments

Assignees

@postscript-dev postscript-dev

Labels
architecture bug imageHandler Anything related to specific ImageHandlers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@postscript-dev @kmilos @1div0 @piponazo