You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tika.apache.org by "tballison (via GitHub)" <gi...@apache.org> on 2026/07/09 20:58:38 UTC
[PR] Follow on tweaks to the recent TIKA-2861 fix [tika]
tballison opened a new pull request, #2938:
URL: https://github.com/apache/tika/pull/2938
Simplify regex Pattern. Follow up to https://github.com/apache/tika/pull/2935
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Follow on tweaks to the recent TIKA-2861 fix [tika]
Posted by "tballison (via GitHub)" <gi...@apache.org>.
tballison merged PR #2938:
URL: https://github.com/apache/tika/pull/2938
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Follow on tweaks to the recent TIKA-2861 fix [tika]
Posted by "tballison (via GitHub)" <gi...@apache.org>.
tballison commented on code in PR #2938:
URL: https://github.com/apache/tika/pull/2938#discussion_r3558252595
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp4/TikaMp4BoxHandler.java:
##########
@@ -186,12 +183,12 @@ private void processQuickTimeItemList(@Nullable byte[] payload) {
* the raw value, so QuickTime location matches the {@code geo:*} output of the udta path.
*/
private void addLocation(String iso6709) {
- Matcher matcher = ISO6709_PATTERN.matcher(iso6709);
- if (matcher.find()) {
- tikaMetadata.set(TikaCoreProperties.LATITUDE, Double.parseDouble(matcher.group(1)));
- tikaMetadata.set(TikaCoreProperties.LONGITUDE, Double.parseDouble(matcher.group(2)));
- if (matcher.group(3) != null) {
- tikaMetadata.set(TikaCoreProperties.ALTITUDE, Double.parseDouble(matcher.group(3)));
+ ISO6709.Location location = ISO6709.parse(iso6709);
+ if (location != null) {
+ tikaMetadata.set(TikaCoreProperties.LATITUDE, location.latitude);
+ tikaMetadata.set(TikaCoreProperties.LONGITUDE, location.longitude);
+ if (location.altitude != null) {
+ tikaMetadata.set(TikaCoreProperties.ALTITUDE, location.altitude);
}
Review Comment:
Y, not clear for coupling reasons? Definitely a separate issue and ticket. As long as altitude is optional, I think this makes a bunch of sense. wdyt? This would allow us to have arrays of points rather than parallel arrays, which would be so much better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Follow on tweaks to the recent TIKA-2861 fix [tika]
Posted by "dschmidt (via GitHub)" <gi...@apache.org>.
dschmidt commented on code in PR #2938:
URL: https://github.com/apache/tika/pull/2938#discussion_r3555022526
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp4/TikaMp4BoxHandler.java:
##########
@@ -186,12 +183,12 @@ private void processQuickTimeItemList(@Nullable byte[] payload) {
* the raw value, so QuickTime location matches the {@code geo:*} output of the udta path.
*/
private void addLocation(String iso6709) {
- Matcher matcher = ISO6709_PATTERN.matcher(iso6709);
- if (matcher.find()) {
- tikaMetadata.set(TikaCoreProperties.LATITUDE, Double.parseDouble(matcher.group(1)));
- tikaMetadata.set(TikaCoreProperties.LONGITUDE, Double.parseDouble(matcher.group(2)));
- if (matcher.group(3) != null) {
- tikaMetadata.set(TikaCoreProperties.ALTITUDE, Double.parseDouble(matcher.group(3)));
+ ISO6709.Location location = ISO6709.parse(iso6709);
+ if (location != null) {
+ tikaMetadata.set(TikaCoreProperties.LATITUDE, location.latitude);
+ tikaMetadata.set(TikaCoreProperties.LONGITUDE, location.longitude);
+ if (location.altitude != null) {
+ tikaMetadata.set(TikaCoreProperties.ALTITUDE, location.altitude);
}
Review Comment:
Wondering if tikaMetadata could be made aware of the ISO6709.Location type? Then 5 lines would shrink to one ... but maybe it's not a good idea for coupling reasons 🤷🏻♂️
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Follow on tweaks to the recent TIKA-2861 fix [tika]
Posted by "dschmidt (via GitHub)" <gi...@apache.org>.
dschmidt commented on code in PR #2938:
URL: https://github.com/apache/tika/pull/2938#discussion_r3555051150
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp4/TikaMp4BoxHandler.java:
##########
@@ -186,12 +183,12 @@ private void processQuickTimeItemList(@Nullable byte[] payload) {
* the raw value, so QuickTime location matches the {@code geo:*} output of the udta path.
*/
private void addLocation(String iso6709) {
- Matcher matcher = ISO6709_PATTERN.matcher(iso6709);
- if (matcher.find()) {
- tikaMetadata.set(TikaCoreProperties.LATITUDE, Double.parseDouble(matcher.group(1)));
- tikaMetadata.set(TikaCoreProperties.LONGITUDE, Double.parseDouble(matcher.group(2)));
- if (matcher.group(3) != null) {
- tikaMetadata.set(TikaCoreProperties.ALTITUDE, Double.parseDouble(matcher.group(3)));
+ ISO6709.Location location = ISO6709.parse(iso6709);
+ if (location != null) {
+ tikaMetadata.set(TikaCoreProperties.LATITUDE, location.latitude);
+ tikaMetadata.set(TikaCoreProperties.LONGITUDE, location.longitude);
+ if (location.altitude != null) {
+ tikaMetadata.set(TikaCoreProperties.ALTITUDE, location.altitude);
}
Review Comment:
FWIW it might come in handy working on this https://issues.apache.org/jira/browse/TIKA-4781?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Follow on tweaks to the recent TIKA-2861 fix [tika]
Posted by "dschmidt (via GitHub)" <gi...@apache.org>.
dschmidt commented on code in PR #2938:
URL: https://github.com/apache/tika/pull/2938#discussion_r3555170062
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp4/TikaMp4BoxHandler.java:
##########
@@ -186,12 +183,12 @@ private void processQuickTimeItemList(@Nullable byte[] payload) {
* the raw value, so QuickTime location matches the {@code geo:*} output of the udta path.
*/
private void addLocation(String iso6709) {
- Matcher matcher = ISO6709_PATTERN.matcher(iso6709);
- if (matcher.find()) {
- tikaMetadata.set(TikaCoreProperties.LATITUDE, Double.parseDouble(matcher.group(1)));
- tikaMetadata.set(TikaCoreProperties.LONGITUDE, Double.parseDouble(matcher.group(2)));
- if (matcher.group(3) != null) {
- tikaMetadata.set(TikaCoreProperties.ALTITUDE, Double.parseDouble(matcher.group(3)));
+ ISO6709.Location location = ISO6709.parse(iso6709);
+ if (location != null) {
+ tikaMetadata.set(TikaCoreProperties.LATITUDE, location.latitude);
+ tikaMetadata.set(TikaCoreProperties.LONGITUDE, location.longitude);
+ if (location.altitude != null) {
+ tikaMetadata.set(TikaCoreProperties.ALTITUDE, location.altitude);
}
Review Comment:
... unless it would become a generic Location class - with `parseFromIso6709()`, `parseSomething()` methods that would also parse other formats as well?
Was never a Java person, so not sure what's really idiomatic :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Follow on tweaks to the recent TIKA-2861 fix [tika]
Posted by "dschmidt (via GitHub)" <gi...@apache.org>.
dschmidt commented on code in PR #2938:
URL: https://github.com/apache/tika/pull/2938#discussion_r3555148518
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp4/TikaMp4BoxHandler.java:
##########
@@ -186,12 +183,12 @@ private void processQuickTimeItemList(@Nullable byte[] payload) {
* the raw value, so QuickTime location matches the {@code geo:*} output of the udta path.
*/
private void addLocation(String iso6709) {
- Matcher matcher = ISO6709_PATTERN.matcher(iso6709);
- if (matcher.find()) {
- tikaMetadata.set(TikaCoreProperties.LATITUDE, Double.parseDouble(matcher.group(1)));
- tikaMetadata.set(TikaCoreProperties.LONGITUDE, Double.parseDouble(matcher.group(2)));
- if (matcher.group(3) != null) {
- tikaMetadata.set(TikaCoreProperties.ALTITUDE, Double.parseDouble(matcher.group(3)));
+ ISO6709.Location location = ISO6709.parse(iso6709);
+ if (location != null) {
+ tikaMetadata.set(TikaCoreProperties.LATITUDE, location.latitude);
+ tikaMetadata.set(TikaCoreProperties.LONGITUDE, location.longitude);
+ if (location.altitude != null) {
+ tikaMetadata.set(TikaCoreProperties.ALTITUDE, location.altitude);
}
Review Comment:
Oh, we're not dealing with ISO6709 there - nvm :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org