Skip to content
New issue

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

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

Already on GitHub? # to your account

Using cached thumbnails for images in messages view #89

Merged
merged 7 commits into from
Apr 26, 2021

Conversation

oxtoacart
Copy link

@oxtoacart oxtoacart commented Apr 25, 2021

This incorporates the latest messaging-android that generates thumbnails and uses those thumbnails in the messages view.

The thumbnails are cached in an in-memory LRU cache.

Note - on debug builds, scrolling will still seem kind of slow (Flutter performance in debug builds is very slow in general) but it should be fine in release builds. We can (and should) do release builds periodically to try out the performance.

BTW - the thumbnail generation seems to mess up the orientation of some images, I'll look into that.

Note - I did not add functionality to open the image in full screen on tap, but I think that would be a useful behavior.

Oh also, the images are pretty pixelated, I'll bump up the resolution of the thumbnails generated in the messaging-android library.

implementation 'com.github.getlantern:libsignal-protocol-java:5c7d676'
implementation 'com.github.getlantern:libsignal-metadata-java:2aed184'
implementation 'com.github.getlantern:messaging-android:890f2897ae'
implementation 'com.github.getlantern:messaging-android:caa2fbcf5f'
Copy link
Author

Choose a reason for hiding this comment

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

The above dependencies are no longer necessary because they're exported by the messaging-android dependency.

@oxtoacart oxtoacart force-pushed the ox/attachment_types branch from cf18c72 to cb8f8c7 Compare April 25, 2021 03:02
@oxtoacart
Copy link
Author

Orientation issue is fixed, thumbnail quality is improved (though I'm not 100% happy with it yet).

Copy link

@cosmicespresso cosmicespresso left a comment

Choose a reason for hiding this comment

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

Just tested this, looks good! Some comments:

  • I made the thumbnails as wide as possible and resolution looks great.
  • How will this handle GIFs and videos? I tested with both: GIF goes through but is thumbnailed as a static image, video doesn't go through (gets posted as an empty message).
  • I totally agree we need to have some sort of UX on double tap/long tap: a full screen attachment viewer but maybe also a way to save the file in a directory of their choice?

I think we can merge this and I will create a separate PR for the wechat example changes.

@oxtoacart
Copy link
Author

  • video doesn't go through (gets posted as an empty message)

I have a unit test that uses an H.264 video but maybe there's a problem with other formats. Do you have a branch with video detection enabled where I could debug this?

@cosmicespresso
Copy link

  • video doesn't go through (gets posted as an empty message)

I have a unit test that uses an H.264 video but maybe there's a problem with other formats. Do you have a branch with video detection enabled where I could debug this?

The Filepicker we are using in this branch can detect videos:
var pickedFile = await FilePicker.platform.pickFiles(allowMultiple: false, type: FileType.any); (is this what you are asking?)

@oxtoacart
Copy link
Author

  • video doesn't go through (gets posted as an empty message)

I have a unit test that uses an H.264 video but maybe there's a problem with other formats. Do you have a branch with video detection enabled where I could debug this?

The Filepicker we are using in this branch can detect videos:
var pickedFile = await FilePicker.platform.pickFiles(allowMultiple: false, type: FileType.any); (is this what you are asking?)

In attachment.dart, I don't see an implementation of Video attachments.

case 'video/*':
    // return Flexible(child: _VideoAttachment(attachment));

@cosmicespresso
Copy link

  • video doesn't go through (gets posted as an empty message)

I have a unit test that uses an H.264 video but maybe there's a problem with other formats. Do you have a branch with video detection enabled where I could debug this?

The Filepicker we are using in this branch can detect videos:
var pickedFile = await FilePicker.platform.pickFiles(allowMultiple: false, type: FileType.any); (is this what you are asking?)

In attachment.dart, I don't see an implementation of Video attachments.

case 'video/*':
    // return Flexible(child: _VideoAttachment(attachment));

Oh I see - no haven't managed to get to that part yet!

@oxtoacart
Copy link
Author

Oh I see - no haven't managed to get to that part yet!

I think that might explain why videos show up as an empty message :)

@cosmicespresso
Copy link

Oh I see - no haven't managed to get to that part yet!

I think that might explain why videos show up as an empty message :)

🤦‍♀️ 🤦‍♀️ 🤦‍♀️ (sorry!)

@oxtoacart
Copy link
Author

Oh I see - no haven't managed to get to that part yet!

I think that might explain why videos show up as an empty message :)

🤦‍♀️ 🤦‍♀️ 🤦‍♀️ (sorry!)

No worries! I'll go ahead and merge this PR. Once we have video thumbnail support in the UI, if there's any issues we can address them then.

@oxtoacart oxtoacart merged commit 318abfc into kr/attachment_types Apr 26, 2021
@oxtoacart oxtoacart deleted the ox/attachment_types branch April 26, 2021 16:37
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants