Opened 7 years ago

Closed 7 years ago

#4783 closed defect (fixed)

Browse can't play Video or Audio made by record

Reported by: AlexL Owned by: chihyu
Priority: blocker Milestone: Update.1
Component: sugar Version:
Keywords: review+, relnote Cc: AlexL, olpc-dev@…, tomeu, cscott, jg, ffm
Blocked By: Blocking:
Deployments affected: Action Needed:
Verified: yes

Description

build joyride 257 q2d04

There was a bug like this, which was fixed, but it has now reappeared.
When you try to resume an audio or video file made by record, browse opens, and says it's downloading the file. After it completes, if you click open, it does nothing.

This may be related to bug http://dev.laptop.org/ticket/4782

Attachments (8)

mime.patch (548 bytes) - added by marco 7 years ago.
mime.2.patch (548 bytes) - added by marco 7 years ago.
shell.log (8.8 KB) - added by chihyu 7 years ago.
rainbow (6.8 KB) - added by chihyu 7 years ago.
org.laptop.WebActivity-2.log (3.9 KB) - added by chihyu 7 years ago.
org.laptop.RecordActivity-1.log (1.2 KB) - added by chihyu 7 years ago.
messages (3.6 KB) - added by chihyu 7 years ago.
message recorded during the period when video was played by Browse
dmesg (25.7 KB) - added by chihyu 7 years ago.

Download all attachments as: .zip

Change History (46)

comment:1 follow-up: Changed 7 years ago by jg

  • Milestone changed from Never Assigned to Update.1
  • Priority changed from high to blocker

Does this work if security turned off?

comment:2 Changed 7 years ago by erikb

  • Owner changed from erikb to jg

Record-39.xo on Trial-3, b625, can successfully save photo+audio+video to the datastore and clipboard and also play those back successfully from the datastore and clipboard.

Thoughts:

  • Has something changed in how the clipboard works since then? What?
  • Maybe audio and video files don't work because of the audio bug on joyride? (Although they can be played back in Record, so I am not sure how this would be effecting things).

Suggestions:

  • Would an activity that just adds a video or audio to the clipboard & datastore be helpful for debugging?

comment:3 in reply to: ↑ 1 Changed 7 years ago by erikb

Replying to jg:

Does this work if security turned off?

Record does not launch with Security turned on.

comment:4 Changed 7 years ago by erikb

  • Cc olpc-dev@… added

comment:5 Changed 7 years ago by tomeu

  • Cc tomeu added
  • Owner changed from jg to marco

The DS fails to add the correct extension to the checked-out file.

The following patch fixes the DS:

diff --git a/src/olpc/datastore/model.py b/src/olpc/datastore/model.py
index 4558a10..cdaee9f 100644
--- a/src/olpc/datastore/model.py
+++ b/src/olpc/datastore/model.py
@@ -14,7 +14,10 @@ import datetime
 import os
 import time
 import warnings
+import logging
+
 from sugar import mime
+
 from olpc.datastore.utils import timeparse
 
 
@@ -227,7 +230,17 @@ class Content(object):
                 else: v = ''
             d[k] = v
         return d
-    
+
+    def _get_extension_from_mimetype(self):
+        # try to get an extension from the mimetype if available
+        mt = self.get_property('mime_type', None)
+        if mt is not None:
+            ext = mime.get_primary_extension(mt)
+            # .ksh is a strange ext for plain text
+            if ext and ext == '.ksh': ext = '.txt'
+            if ext and ext == '.jpe': ext = '.jpg' # fixes #3163
+            return ext
+        return None
 
     def suggestName(self):
         # we look for certain known property names
@@ -237,6 +250,10 @@ class Content(object):
         # checkout name
         filename = self.get_property('filename', None)
         ext = self.get_property('ext', '')
+        if not ext:
+            ext = self._get_extension_from_mimetype()
+
+        logging.debug('Content.suggestName: %r %r' % (filename, ext))
 
         if filename:
             # some backingstores keep the full relative path
@@ -244,16 +261,9 @@ class Content(object):
             f, e = os.path.splitext(filename)
             if e: return filename, None
             if ext: return "%s.%s" % (filename, ext), None
-        elif ext: return None, ext
-        else:
-            # try to get an extension from the mimetype if available
-            mt = self.get_property('mime_type', None)
-            if mt:
-                ext = mime.get_primary_extension(mt)
-                # .ksh is a strange ext for plain text
-                if ext and ext == '.ksh': ext = '.txt'
-                if ext and ext == '.jpe': ext = '.jpg' # fixes #3163
-                if ext: return None, ext
+        elif ext:
+            return None, ext
+
         return None, None
 
     def get_file(self):

but that's not enough to make it work, as the globs files installed in the xo doesn't contain an entry for video/ogg.

If the patch is applied, an entry like below was added to the globs file and the plugin is configured to open the ogv extension, then it should work.

video/ogg:*.ogv

Passing to Marco to figure out the mimetypes issue.

comment:6 Changed 7 years ago by marco

  • Owner changed from marco to tomeu

Back to Tomeu to get the DS patch in. We will have to discuss the mime issue to decide what to do for Update.1 and what later. audio/ogg and video/ogg are not currently supported by the system.

comment:7 Changed 7 years ago by tomeu

  • Owner changed from tomeu to ApprovalForUpdate

Asking for permission to push that patch.

comment:8 Changed 7 years ago by jg

  • Owner changed from ApprovalForUpdate to cscott

approved.

comment:9 Changed 7 years ago by tomeu

  • Owner changed from cscott to marco

Pushed. Marco, can you make an rpm and then assign to Dennis/Scott for tagging?

comment:10 Changed 7 years ago by marco

  • Cc cscott jg added
  • Owner changed from marco to dgilmore

Dennis please push this rpm to Update.1:

sugar-datastore-0.2.2-0.38.20071112git.6d3d607ec7

comment:11 Changed 7 years ago by marco

When that's done please just reassign back to me, we still need to solve the specific mime type issue.

comment:12 Changed 7 years ago by marco

  • Keywords review? added

Changed 7 years ago by marco

Changed 7 years ago by marco

comment:13 Changed 7 years ago by tomeu

  • Keywords review+ added; review? removed

comment:14 Changed 7 years ago by marco

  • Component changed from camera-activity to sugar

comment:15 Changed 7 years ago by marco

Checked in on master

comment:16 Changed 7 years ago by cscott

  • Owner changed from dgilmore to cscott
  • Status changed from new to assigned

I'm managing the update.1 repos. Please leave these bugs assigned to me unless I reassign to Dennis, thanks.

How does this interact with the recently reverted sugar-datastore-0.3? Does the new 0.2 rpm contain anything other than this patch? Has this package been tested in joyride?

comment:17 Changed 7 years ago by marco

The change is in sugar-base. The datastore use this API but there is no relation with the reverted 0.3. It's not built in joyride yet.

What's the status of joyride btw? It doesn't seem to be hourly building...

comment:18 Changed 7 years ago by marco

Oh I think you are confused... The rpm referenced in comment 10 went in joyride a while ago (and so it was tagged for Update.1). mime.2.patch, which is what the ticket is still open for, is not yet in a build.

comment:19 Changed 7 years ago by marco

  • Owner changed from cscott to marco
  • Status changed from assigned to new

The ticket should be assigned to me so that I actually get the patch into a joyride build and then I ask for approval...

comment:20 Changed 7 years ago by marco

  • Owner changed from marco to ApprovalForUpdate

Seem to be working fine in 332

comment:21 Changed 7 years ago by jg

  • Owner changed from ApprovalForUpdate to cscott

OK for Update.1

comment:22 Changed 7 years ago by marco

  • Owner changed from cscott to marco

I need to build an rpm fo Update-1. Please always assign back to me after Approval, unless I mention the nvr of the rpm to push...

comment:23 Changed 7 years ago by marco

Fedora package cvs seem to have problems atm, I'll build it when it comes back.

comment:24 Changed 7 years ago by marco

  • Owner changed from marco to cscott

Package built:
sugar-base-0.1.1-1

comment:25 Changed 7 years ago by kimquirk

  • Keywords relnote added

comment:26 Changed 7 years ago by AlexL

  • Resolution set to fixed
  • Status changed from new to closed
  • Verified set

verified as fixed in joyride 345

comment:27 Changed 7 years ago by marco

  • Resolution fixed deleted
  • Status changed from closed to reopened

This should stay open so that cscott gets it into Update.1... Thanks for testing!

comment:28 Changed 7 years ago by marco

  • Owner changed from cscott to dgilmore
  • Status changed from reopened to new

Looks like Dennis is now managing Update.1

comment:29 Changed 7 years ago by marco

As a reminder, the package to get in here is sugar-base-0.1.1-1

comment:30 Changed 7 years ago by ffm

  • Cc ffm added
  • Component changed from sugar to browse-activity

comment:31 Changed 7 years ago by ffm

  • Component changed from browse-activity to sugar

comment:32 Changed 7 years ago by marco

  • Resolution set to fixed
  • Status changed from new to closed

Verified fixed in 657

comment:33 Changed 7 years ago by chihyu

  • Resolution fixed deleted
  • Status changed from closed to reopened

Tested on joyride 1537 with firmware Q2D08, and the same problem occurred.

Is the patch included in the joyride builds as well, or just in Update.1?

comment:34 follow-up: Changed 7 years ago by marco

Works fine for me in 1537. Can you provide the exact steps to reproduce and logs please?
http://wiki.laptop.org/go/Attaching_Sugar_Logs_to_Tickets

comment:35 Changed 7 years ago by marco

  • Owner changed from dgilmore to chihyu
  • Status changed from reopened to new

comment:36 in reply to: ↑ 34 Changed 7 years ago by chihyu

It now works for me in 1537, but I do find some warning messages in the log files. Please see the attachment.

Replying to marco:

Works fine for me in 1537. Can you provide the exact steps to reproduce and logs please?
http://wiki.laptop.org/go/Attaching_Sugar_Logs_to_Tickets

Changed 7 years ago by chihyu

Changed 7 years ago by chihyu

Changed 7 years ago by chihyu

Changed 7 years ago by chihyu

Changed 7 years ago by chihyu

message recorded during the period when video was played by Browse

Changed 7 years ago by chihyu

comment:37 Changed 7 years ago by tomeu

Could you please give it some more testing and close it if things are working now? Thanks.

comment:38 Changed 7 years ago by marco

  • Resolution set to fixed
  • Status changed from new to closed

Let's close it. Please open a separate ticket if you see it again and attach logs. (The warnings in the logs you posted are "normal" according to Erik).

Note: See TracTickets for help on using tickets.